Xuanwo merged PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.ap
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2209086861
##
test/in_memory_catalog_test.cc:
##
@@ -58,6 +75,21 @@ TEST_F(InMemoryCatalogTest, TableExists) {
EXPECT_THAT(result, HasValue(::testing::Eq(false)));
}
+TEST
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204242342
##
test/temp_file_test_base.h:
##
@@ -31,6 +31,46 @@
namespace iceberg {
+/// \brief Get the test name for inclusion in the filename
Review Comment:
I notice
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204247772
##
test/in_memory_catalog_test.cc:
##
@@ -58,6 +75,21 @@ TEST_F(InMemoryCatalogTest, TableExists) {
EXPECT_THAT(result, HasValue(::testing::Eq(false)));
}
+TEST
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2206472958
##
src/iceberg/catalog.h:
##
@@ -166,8 +166,7 @@ class ICEBERG_EXPORT Catalog {
/// \param identifier a table identifier
/// \return instance of Table implement
wgtmac commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2206437585
##
src/iceberg/catalog.h:
##
@@ -166,8 +166,7 @@ class ICEBERG_EXPORT Catalog {
/// \param identifier a table identifier
/// \return instance of Table implementa
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2206242877
##
src/iceberg/catalog/in_memory_catalog.cc:
##
@@ -440,44 +376,60 @@ Result>
InMemoryCatalogImpl::ListTables(
return table_idents;
}
-Result> InMemoryCatalogI
gty404 commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2205059030
##
src/iceberg/catalog/in_memory_catalog.cc:
##
@@ -440,44 +376,60 @@ Result>
InMemoryCatalogImpl::ListTables(
return table_idents;
}
-Result> InMemoryCatalogIm
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204247772
##
test/in_memory_catalog_test.cc:
##
@@ -58,6 +75,21 @@ TEST_F(InMemoryCatalogTest, TableExists) {
EXPECT_THAT(result, HasValue(::testing::Eq(false)));
}
+TEST
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204242342
##
test/temp_file_test_base.h:
##
@@ -31,6 +31,46 @@
namespace iceberg {
+/// \brief Get the test name for inclusion in the filename
Review Comment:
Update t
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204230254
##
src/iceberg/catalog/in_memory_catalog.cc:
##
@@ -441,44 +376,58 @@ Result>
InMemoryCatalogImpl::ListTables(
return table_idents;
}
-Result> InMemoryCatalogI
lishuxu commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2204220852
##
src/iceberg/catalog/in_memory_catalog.cc:
##
@@ -441,44 +376,58 @@ Result>
InMemoryCatalogImpl::ListTables(
return table_idents;
}
-Result> InMemoryCatalogI
wgtmac commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2191952179
##
src/iceberg/catalog/in_memory_catalog.cc:
##
@@ -441,44 +376,58 @@ Result>
InMemoryCatalogImpl::ListTables(
return table_idents;
}
-Result> InMemoryCatalogIm
wgtmac commented on code in PR #142:
URL: https://github.com/apache/iceberg-cpp/pull/142#discussion_r2191757667
##
test/temp_file_test_base.h:
##
@@ -31,6 +31,46 @@
namespace iceberg {
+/// \brief Get the test name for inclusion in the filename
Review Comment:
Can we re
lishuxu opened a new pull request, #142:
URL: https://github.com/apache/iceberg-cpp/pull/142
Note: Since the LoadTable interface needs to return a Table object that
holds a shared_from_this pointer to the catalog, I remove InMemoryCatalog
inheritance from Catalog and instead directly implem
15 matches
Mail list logo
