Remove the kTeapot table type - #77
Open
andr-sokolov wants to merge 4 commits into
Open
Conversation
Remove the corresponding TeapotTable structure. Remove branches where TeapotTable is used. Remove functions, functions arguments and structures which has become unused.
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy kTeapot table type end-to-end (table config structure, metadata access path, and teapot-specific smoke tests), and simplifies CI/smoke-test configuration accordingly.
Changes:
- Removes
TeapotTable/kTeapotfrom table source/config parsing and samovar planning utilities. - Deletes teapot metadata access implementation (
access_teapot.*) and removes/adjusts teapot-only smoke tests and flags. - Updates smoke-test CI matrix and runner scripts to no longer accept
--metadata_type.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tea/table/ut/config_test.cpp | Drops teapot URL parsing expectations; keeps coverage for file/special/iceberg URL forms. |
| tea/smoke_test/test_base.h | Simplifies smoke-test setup to always use IcebergMetadataWriterBuilder. |
| tea/smoke_test/teapot_test_base.h | Removes teapot-only test harness. |
| tea/smoke_test/stats_test.cpp | Removes teapot-only stats tests/branches; normalizes expectations to iceberg-only. |
| tea/smoke_test/spark_generated_test_base.h | Removes metadata-type gating so generated tests run under the remaining metadata path. |
| tea/smoke_test/samovar_limit_test.cpp | Removes metadata-type gating for samovar-only test setup. |
| tea/smoke_test/run_tests.cpp | Removes --metadata_type flag wiring. |
| tea/smoke_test/real_parquet_test.cpp | Deletes teapot-only “real parquet” smoke test. |
| tea/smoke_test/positional_delete_test.cpp | Removes teapot-only positional delete scenarios/includes. |
| tea/smoke_test/perf_test/positional_delete_test.cpp | Removes no-longer-needed metadata-type state from perf test fixture. |
| tea/smoke_test/part_of_file_test.cpp | Deletes teapot-only “part of file” smoke tests. |
| tea/smoke_test/negative_server_test.cpp | Removes teapot-only negative tests/branches; keeps iceberg-path negatives. |
| tea/smoke_test/name_mapping_default_test.cpp | Removes metadata-type skip so the test always runs on the remaining path. |
| tea/smoke_test/large_metadata_test.cpp | Removes metadata-type gating (assumes iceberg-only). |
| tea/smoke_test/iceberg_test.cpp | Removes a test that depended on teapot error behavior. |
| tea/smoke_test/filter_tests/filter_test_base.cpp | Removes teapot-only assertions against teapot request filter JSON. |
| tea/smoke_test/equality_delete_test.cpp | Removes teapot-only equality delete scenarios/includes. |
| tea/smoke_test/environment.h | Removes MetadataType from smoke-test environment API. |
| tea/smoke_test/environment.cpp | Removes metadata-type parsing/state from environment implementation. |
| tea/smoke_test/deleted_files_test.cpp | Removes metadata-type gating so the test always runs on the remaining path. |
| tea/smoke_test/delete_test.cpp | Removes teapot-only delete scenario/include. |
| tea/smoke_test/complex_queries_test.cpp | Deletes teapot-only complex query smoke test. |
| tea/smoke_test/CMakeLists.txt | Removes deleted smoke-test sources from the build. |
| tea/samovar/utils.cpp | Removes TeapotTable handling from session identifier generation. |
| tea/samovar/planner.cpp | Removes TeapotTable handling from coordinator selection logic. |
| tea/metadata/CMakeLists.txt | Drops teapot metadata access sources from the metadata library. |
| tea/metadata/access_teapot.h | Deletes teapot metadata access API. |
| tea/metadata/access_teapot.cpp | Deletes teapot metadata access implementation (grpc/teapot path). |
| tea/gpext/tea_reader.cpp | Removes teapot metadata planning/access path and related plumbing. |
| tea/common/utils.h | Removes teapot metadata conversion declaration that was only used by teapot access. |
| tea/common/utils.cpp | Removes teapot response-to-scan-metadata conversion implementation. |
| tea/common/config.h | Removes TeapotTable from TableSource and drops kTeapot from TableType. |
| tea/common/config.cpp | Removes teapot URL parsing branch from ConfigSource::GetTableConfig(). |
| ci/run-smoke.sh | Updates smoke runner invocation to drop metadata type argument. |
| .github/workflows/smoke-tests.yml | Simplifies smoke-test matrix and artifact naming to remove metadata type dimension. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
705
to
707
| const auto schema = | ||
| (components.schema.empty()) ? table_config.config.meta_access.default_schema : std::string(components.schema); | ||
|
|
Comment on lines
53
to
57
| auto maybe_result = pq::TableScanQuery(kDefaultTableName, column_names).SetWhere(condition).Run(*conn_); | ||
| if (expected.is_error) { | ||
| ASSERT_FALSE(maybe_result.ok()); | ||
| } else { | ||
| ASSERT_TRUE(maybe_result.ok()) << maybe_result.status().message(); |
Comment on lines
234
to
+236
| // TeapotMetadataWriter DOES NOT split result by row groups | ||
| // IcebergMetadataWriter DOES split result by row groups | ||
| const int32_t tasks_after_splitting = Environment::GetMetadataType() == MetadataType::kIceberg ? 3 : 1; | ||
| const int32_t tasks_after_splitting = 3; |
Comment on lines
+20
to
22
| if (Environment::GetProfile() != "samovar") { | ||
| GTEST_SKIP() << "Skip test only for iceberg with Samovar profile"; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the corresponding TeapotTable structure. Remove branches where
TeapotTable is used. Remove functions, functions arguments and structures which
has become unused.
Remove tests related to Teapot. Don't pass metadata type to smoke_test. Assume
that the metadata type is always not kTeapot. Delete the test code that is
executed when metadata type is kTeapot.