Skip to content

Support parameters in table function arguments#12910

Merged
kasiafi merged 1 commit intotrinodb:masterfrom
kasiafi:368EvaluateConstant
Jun 21, 2022
Merged

Support parameters in table function arguments#12910
kasiafi merged 1 commit intotrinodb:masterfrom
kasiafi:368EvaluateConstant

Conversation

@kasiafi
Copy link
Copy Markdown
Member

@kasiafi kasiafi commented Jun 20, 2022

This is an improvement. It allows you to use parameters in table function scalar arguments:

PREPARE stmt1 FROM 
SELECT * FROM TABLE(postgresql.system.query(query => ?));

EXECUTE stmt1 USING 'SELECT * FROM tpch.region';

Documentation

Existing documentation is updated in this PR. Some examples (like the above) should be added to the documentation of the query pass-through table functions #12725 cc @colebow @mosabua

edit: @colebow @mosabua, I removed the second example, which included query => format('SELECT * FROM %s', ?). As @martint noticed (#12910 (comment)), it is the kind of usage we shouldn't recommend in the docs.

Release notes

# General
* Support parameters in table function arguments.

@cla-bot cla-bot bot added the cla-signed label Jun 20, 2022
@kasiafi kasiafi force-pushed the 368EvaluateConstant branch from 88093f9 to ed683fb Compare June 20, 2022 13:15
@sajjoseph
Copy link
Copy Markdown
Contributor

Thanks @kasiafi for this fix. It will go a long way to make PTF query pushdown work for regular use cases.

@github-actions github-actions bot added the docs label Jun 20, 2022
@kasiafi kasiafi force-pushed the 368EvaluateConstant branch from ed683fb to f0508a7 Compare June 20, 2022 16:14
@kasiafi kasiafi requested a review from martint June 20, 2022 16:24
@martint
Copy link
Copy Markdown
Member

martint commented Jun 20, 2022

The second example is not something we should generally recommend. It’s a vector for potential sql injection.

@kasiafi
Copy link
Copy Markdown
Member Author

kasiafi commented Jun 20, 2022

The second example is not something we should generally recommend. It’s a vector for potential sql injection.

I don't think we're going to prevent such usage. Maybe we should add a warning in the docs?

@martint
Copy link
Copy Markdown
Member

martint commented Jun 20, 2022

No, we’re not, but we shouldn’t provide such an example in the docs

@kasiafi kasiafi merged commit 0d2dc80 into trinodb:master Jun 21, 2022
@github-actions github-actions bot added this to the 387 milestone Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants