Skip to content

Complete TVF Port#25032

Closed
mohsaka wants to merge 30 commits into
prestodb:masterfrom
mohsaka:tvf_cleanup
Closed

Complete TVF Port#25032
mohsaka wants to merge 30 commits into
prestodb:masterfrom
mohsaka:tvf_cleanup

Conversation

@mohsaka

@mohsaka mohsaka commented May 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Add TVF implementation into Presto, ported from Trino for consistency.
Prestissimo implementation is currently in the works.

Documentation and E2E Prestissmo testing will be added in a future PR. We will add some more improvements in the future but Trino's documentation can be followed currently until we diverge.

https://trino.io/docs/current/functions/table.html
https://trino.io/docs/current/develop/table-functions.html

Motivation and Context

We want to include table function support in Prestissimo. To allow for this we need support in Presto.

Impact

Test Plan

TestTableFunctionInvocation tests as well as Parser and Analysis tests.

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
* ... Table Function Support

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label May 2, 2025
@mohsaka mohsaka mentioned this pull request May 2, 2025
Merged
6 tasks
@mohsaka mohsaka force-pushed the tvf_cleanup branch 3 times, most recently from 0af0064 to aea30cc Compare May 7, 2025 15:45
@mohsaka mohsaka force-pushed the tvf_cleanup branch 2 times, most recently from 7cc7e90 to 59186db Compare May 9, 2025 20:27
@mohsaka mohsaka marked this pull request as ready for review May 9, 2025 21:05
@mohsaka mohsaka requested a review from ZacBlanco May 9, 2025 21:05
@prestodb-ci prestodb-ci requested review from a team, nmahadevuni and pratyakshsharma and removed request for a team May 9, 2025 21:05
@github-actions

github-actions Bot commented May 9, 2025 •
edited
Loading

Copy link
Copy Markdown

Codenotify: Notifying subscribers in CODENOTIFY files for diff 369605c...8b21f10.

Notify File(s)
@aditi-pandit presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@elharo presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@kaikalur presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@rschlussel presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4

This was referenced May 21, 2025
Closed
Closed
mohsaka and others added 4 commits June 4, 2025 11:05
@mohsaka @kasiafi
Support table function invocation in grammar and AST
cb43449 
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>
@mohsaka @kasiafi
Add SPI support for declaring and analyzing TableFunctions
736b758 
Changes adapted from trino/PR#11336, 12350, 12407, 12476, 12531, 12813
Original commits:
395fd91c6480a993241eeabd9599873d0d05b24b
998315075343beecef962657b8cbf440d53cc13b
712b8e98ff8a726f95295ac539159fc532628273
131dc44af97b31a2fa8115028d98d06671641bfa
0da095e14b0855f89af3c4f254a5a60280fc7170
5310671f80291394b12ba2ea746e4e60051aaff4
18bb60262cb0850cf839c2b20b434344921f5122
4a7d72afb64f93a9748a4c6b4defc2d42bbae000
Author: kasiafi

Modifications were made to adapt to Presto including:
Removal of ConnectorExpression.

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi
Pass TransactionManager to StatementAnalyzer
41b6b11 
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>
@mohsaka @kasiafi
Register table functions
68093b5 
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>
mohsaka and others added 17 commits June 4, 2025 13:28
@mohsaka @kasiafi @xin-zhang2
Analyze table and descriptor arguments
dd1aa95 
Changes adapted from trino/PR#13602
Original commit: dbef4d9be37494967496230573ab400e54aab0d9
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Co-authored-by: Xin Zhang <desertsxin@gmail.com>
@xin-zhang2 @mohsaka
Add TableArgumentAnalysis and fix test case errors
3dcad55 
Co-authored-by: mohsaka <135669458+mohsaka@users.noreply.github.com>
@mohsaka @kasiafi
Analyze table function invocation in the context of a query
0295588 
Changes adapted from trino/PR#13653
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @kasiafi @mohsaka
Add columns from input tables to table function returned relation type
0ca1844 
Changes adapted from trino/PR#14115
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @kasiafi @mohsaka
Plan table function invocation with table arguments
3eb22a6 
Changes adapted from trino/PR#14175
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi
Analyze input columns required by a table function
444e07e 
Changes adapted from trino/PR#15256
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @mohsaka @kasiafi
Implement table function source
b5cd6f6 
Changes adapted from trino/PR#14566
Author: kasiafi

Co-authored-by: mohsaka <135669458+mohsaka@users.noreply.github.com>
Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @mohsaka @kasiafi
Support execution by operator for table functions
65d8121 
Changes adapted from trino/PR#15575
Author: kasiafi

Co-authored-by: mohsaka <135669458+mohsaka@users.noreply.github.com>
Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @kasiafi @mohsaka
Optimize table function source
d6ca377 
Changes adapted from trino/PR#16010,15653
Original commits:
1ee7785548e3cfe9149736e1d0d3176083ab4019
863d2fceb95097068e8f5378adbaca8434b892a8
Author: kasiafi

Optimize plans involving table functions
Fix incorrect input columns passed to table function

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi @xin-zhang2
Optimize plans involving table functions
bb2bc56 
Changes adapted from trino/PR#16012
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
Co-authored-by: Xin Zhang <desertsxin@gmail.com>
@mohsaka @kasiafi
Fix potential ambiguity while parsing queries involving table functions
cf44739 
Changes adapted from trino/PR#16014
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi
Enforce non-empty returned relation type of table function
c1cb9cd 
Changes adapted from trino/PR#16058
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi
Add table function exclude_columns
658d227 
Changes adapted from trino/PR#16584
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @kasiafi
Add table function sequence
7b65eed 
Changes adapted from trino/PR#16716
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @kasiafi @mohsaka
Remove function name from TableFunctionHandle
32df5e9 
Changes adapted from trino/PR#17117
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@xin-zhang2 @kasiafi @mohsaka
Rewrite exclude_columns table function into projection
dddb777 
Changes adapted from trino/PR#25493
Author: kasiafi

Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com>
@mohsaka @xin-zhang2
Cleanup code for PR
d8c3f3d 
Co-authored-by: Xin Zhang <desertsxin@gmail.com>
@xin-zhang2 xin-zhang2 mentioned this pull request Jun 16, 2025
Closed
6 tasks
@mohsaka mohsaka mentioned this pull request Jul 3, 2025
Closed
6 tasks
@steveburnett steveburnett mentioned this pull request Jul 17, 2025
Closed
@steveburnett

steveburnett commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Documentation and E2E Prestissmo testing will be added in a future PR. We will add some more improvements in the future but Trino's documentation can be followed currently until we diverge.

https://trino.io/docs/current/functions/table.html https://trino.io/docs/current/develop/table-functions.html

Opened #25559 for this future work.

@mohsaka mohsaka mentioned this pull request Aug 14, 2025
Merged
6 tasks
@aditi-pandit

aditi-pandit commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

We are splitting this PR into smaller pieces.

This was referenced Aug 18, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shrinidhijoshi shrinidhijoshi Awaiting requested review from shrinidhijoshi shrinidhijoshi is a code owner

@jaystarshot jaystarshot Awaiting requested review from jaystarshot jaystarshot is a code owner

@feilong-liu feilong-liu Awaiting requested review from feilong-liu feilong-liu is a code owner

@elharo elharo Awaiting requested review from elharo elharo is a code owner

@vivek-bharathan vivek-bharathan Awaiting requested review from vivek-bharathan

@ZacBlanco ZacBlanco Awaiting requested review from ZacBlanco ZacBlanco is a code owner automatically assigned from prestodb/committers

@nmahadevuni nmahadevuni Awaiting requested review from nmahadevuni nmahadevuni was automatically assigned from prestodb/ibm-reviewers

@pratyakshsharma pratyakshsharma Awaiting requested review from pratyakshsharma pratyakshsharma was automatically assigned from prestodb/ibm-reviewers

Assignees

@mohsaka mohsaka

@xin-zhang2 xin-zhang2

Labels

from:IBM PR from IBM

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mohsaka @steveburnett @aditi-pandit @xin-zhang2 @prestodb-ci