chore: Re-add inheritance of Presto macros for Trino et al. #22435
Conversation
john-bodley
changed the title
Codecov Report
@@ Coverage Diff @@
## master #22435 +/- ##
===========================================
- Coverage 66.90% 55.72% -11.18%
===========================================
Files 1850 1850
Lines 70692 70687 -5
Branches 7752 7752
===========================================
- Hits 47294 39388 -7906
- Misses 21382 29283 +7901
Partials 2016 2016
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| def test_extra_table_metadata(self): | ||
| db = mock.Mock() | ||
| db.get_indexes = mock.Mock( | ||
| return_value=[{"column_names": ["ds", "hour"], "name": "partition"}] | ||
| ) | ||
| db.get_extra = mock.Mock(return_value={}) | ||
| db.has_view_by_name = mock.Mock(return_value=None) | ||
| db.get_df = mock.Mock( | ||
| return_value=pd.DataFrame({"ds": ["01-01-19"], "hour": [1]}) | ||
| ) | ||
| result = TrinoEngineSpec.extra_table_metadata(db, "test_table", "test_schema") | ||
| assert result["partitions"]["cols"] == ["ds", "hour"] | ||
| assert result["partitions"]["latest"] == {"ds": "01-01-19", "hour": 1} |
There was a problem hiding this comment.
nit: we should add new db engine spec tests to tests/unit_tests/db_engine_specs/test_*.py. But I can port over this test once this PR is merged, as I'm going to be doing a few changes to the Trino spec to implement some recent features from the new Trino driver.
| @@ -311,6 +311,203 @@ def get_function_names(cls, database: Database) -> List[str]: | |||
| """ | |||
| return database.get_df("SHOW FUNCTIONS")["Function"].tolist() | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
This entire block was simply moved from PrestoEngineSpec to PrestoBaseEngineSpec.
| metadata["partitions"] = { | ||
| "cols": cols, | ||
| "cols": sorted(indexes[0].get("column_names", [])), |
There was a problem hiding this comment.
This is the same logic as before, though now it's inlined. The sorted(...) is to aid with testing.
| latest_parts = tuple([None] * len(col_names)) | ||
|
|
||
| metadata["partitions"] = { | ||
| "cols": sorted( |
There was a problem hiding this comment.
Very similar logic to before except the list is deduped and then sorted to aid with testing.
mistercrunch
added
🏷️ bot
SUMMARY
This PR re-addresses some of the issues #20729 tried to fix which was closed in favor of #21066 (which introduced a few regressions). Specifically it:
latest_partitionandlatest_sub_partitionfunctions (and their dependencies)—which are exposed as macros—from thePrestoEngineSpecto thePrestoBaseEngineSpecso they can be leveraged by theTrinoEngineSpec.TrinoEngineSpec.extra_table_metadatareturn value to include the latest partition spec, thus ensuing that the latest partition information is rendered in the left-hand panel. See the attached screenshot for details as previously this simply statedlatest partition:.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Added unit tests and end-to-end testing.
ADDITIONAL INFORMATION
cc: @dungdm93