Skip to content

Support table functions with pushdown to connector #1#3

Open
mohsaka wants to merge 10 commits into
tvf_allfrom
tvf_11336
Open

Support table functions with pushdown to connector #1#3
mohsaka wants to merge 10 commits into
tvf_allfrom
tvf_11336

Conversation

@mohsaka
Copy link
Copy Markdown
Owner

@mohsaka mohsaka commented Mar 5, 2025

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ... 

If release note is NOT required, use:

== NO RELEASE NOTE ==

@mohsaka mohsaka force-pushed the tvf_11336 branch 3 times, most recently from 5a04797 to ed17f13 Compare March 5, 2025 17:35
@mohsaka mohsaka closed this Mar 5, 2025
@mohsaka mohsaka reopened this Mar 5, 2025
@mohsaka mohsaka force-pushed the tvf_11336 branch 8 times, most recently from 67001c7 to b563ac8 Compare March 7, 2025 20:39
mohsaka and others added 5 commits March 7, 2025 12:40
Changes adapted from trino/PR#11336
Original commit: 2e00c8e64c32d6fdd813999b2e04b3b3415235c8
Author: kasiafi

Modifications were made to adapt to Presto including:
Addition of KEEP in the parser.
Adjustment of the TestSqlParser.java to apply to Presto concepts.
Switch from Trino's DataType based datatypes to Presto's String based datatypes.

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: 395fd91c6480a993241eeabd9599873d0d05b24b
Author: kasiafi

Modifications were made to adapt to Presto including:
Removal of ConnectorExpression. Will be required in a future commit.

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: e69a052e570718ca568114e61644946feab4383e
Author: kasiafi

Modifications were made to adapt to Presto including:
Removal of the StatementAnalyzerFacotry calls.
Add TransactionManager to all StatementAnalyzer constructor calls.

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: 493f639a9daa5e6aaadbb4364587196cb5240fc2
Author: kasiafi

Modifications were made to adapt to Presto including:
Addition of TableFunctionRegistry into FunctionAndTypeManager
Removal of FunctionResolver
Addition of toPath to TableFunctionRegistry

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: 9e8d51ad45f57267f5f7fa6bf8e8c4ec56103dda
Author: kasiafi

Modifications were made to adapt to Presto including:
Removal of Node Location from TrinoException
Added new SemanticErrorCodes
Changed Void context to SqlPlannerContext in RelationPlanner.java
Add newUnqualified to Field class with Presto specification
Add getCanonicalValue to Identifier.java

Co-authored-by: Pratik Joseph Dabre <pdabre12@gmail.com>
Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
mohsaka and others added 4 commits March 7, 2025 12:49
Changes adapted from trino/PR#11336
Original commit: d4c73389bbdb6b48c24a0969b259286b05a99ade
Author: kasiafi

Modifications were made to adapt to Presto including:
Change CatalogName to ConnectorId
Modified and removed outputSymbols -> VariableReferenceExpression
Following visitTable example, created and used VariableReferenceExpression List Builder.
TableFunctionNode extends InternalPlanNode instead of PlanNode. Circular dependency and following other Node classes.

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: 565700985baff0c4b29fdb1e3e26139a29318b9e
Author: kasiafi

Modifications were made to adapt to Presto including:
Add applyTableFunction to all implementations of Metadata
Change CatalogName to ConnectorId
Add empty ConnectorTableLayoutHandle to TableHandle in MetadataManger::applyTableFunction

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Changes adapted from trino/PR#11336
Original commit: ec8b9fd2b2cc9c8bc78c0ca1317dc34fcf2c48c7
Author: kasiafi

Modifications were made to adapt to Presto including:
Change Symbol to VariableReferenceExpression
Add additional arguments to TableScanNode calls
Removal of PlannerContext and replaced with Metadata

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Preparatory commit for supporting table functions in JDBC connectors

Changes adapted from trino/PR#11336
Original commit: b4d4b5102e878b7e38e13c0440432543a18f913e
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka mohsaka changed the base branch from master to tvf_all March 7, 2025 20:52
@mohsaka mohsaka changed the title TVF Support table functions with pushdown to connector #1 Mar 7, 2025
xin-zhang2 pushed a commit that referenced this pull request Nov 10, 2025
…b#26529)

Summary:
1. Fix an error of refresh MV after base table insertion
2. Use ExpressionUtil in MV refresh query composing
3. Fix an issue in MV caused by copying catalog properties during
session creation

For #3, we need a follow up to understand why the check of setting
transaction id and catalog properties during session constructor is
needed. I don't see a clear reason by checking the previous relavant
commits. I'll check with some additional people. If no clear objection,
I'll remove that check at least from the Session constructor in
following PRs, and restoring the copy of catalog properties in
buildOwnerSession().

Differential Revision: D86223888

## Release Notes

```
== NO RELEASE NOTE ==
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant