Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[HIVE-23951] Support parameterized queries in WHERE/HAVING clause #1315

Merged
merged 9 commits into from
Aug 6, 2020
Merged

[HIVE-23951] Support parameterized queries in WHERE/HAVING clause #1315

merged 9 commits into from
Aug 6, 2020

Conversation

vineetgarg02
Copy link
Contributor

@vineetgarg02 vineetgarg02 commented Jul 25, 2020 •
edited

HIVE-23951 : This pull request provides support for PREPARE/EXECUTE statements and ability to use parameters in WHERE/HAVING clause

@vineetgarg02 vineetgarg02 mentioned this pull request Jul 25, 2020
Closed
@vineetgarg02 vineetgarg02 changed the title [DRAFT] Support for prepare/execute statements [HIVE-23951] Support parameterized queries in WHERE/HAVING clause Jul 29, 2020
@vineetgarg02 vineetgarg02 self-assigned this Jul 29, 2020
@vineetgarg02
Copy link
Contributor Author

@jcamachor @zabetak I was thinking of changing all the existing tpcds queries to replace literals with ? and run explain on tpcds queries (using TestTezPerfCliDriver). This should provide us with some good test coverage. What I am undecided on is if it worth pushing it to hive repo and have a separate clidriver testing execute/prepare for tpcds. What do you suggest?

@zabetak
Copy link
Contributor

zabetak commented Jul 30, 2020

@jcamachor @zabetak I was thinking of changing all the existing tpcds queries to replace literals with ? and run explain on tpcds queries (using TestTezPerfCliDriver). This should provide us with some good test coverage. What I am undecided on is if it worth pushing it to hive repo and have a separate clidriver testing execute/prepare for tpcds. What do you suggest?

If we want to avoid regressions I guess having the queries in the repo makes sense. Apart from that I guess that not all of the tpcds queries are needed to ensure code coverage. Possibly after testing you will end-up with a subset that is sufficient.

@jcamachor
Copy link
Contributor

jcamachor commented Aug 2, 2020 •
edited

@jcamachor @zabetak I was thinking of changing all the existing tpcds queries to replace literals with ? and run explain on tpcds queries (using TestTezPerfCliDriver). This should provide us with some good test coverage. What I am undecided on is if it worth pushing it to hive repo and have a separate clidriver testing execute/prepare for tpcds. What do you suggest?

Why do we need a new driver though? We should probably add it to the existing one.

jcamachor
jcamachor reviewed Aug 3, 2020
View reviewed changes
parser/src/java/org/apache/hadoop/hive/ql/parse/HiveLexerParent.g Show resolved Hide resolved
parser/src/java/org/apache/hadoop/hive/ql/parse/PrepareStatementParser.g Show resolved Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/Compiler.java Outdated Show resolved Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/Driver.java Outdated Show resolved Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java Outdated Show resolved Hide resolved
ql/src/test/results/clientpositive/llap/prepare_plan.q.out Show resolved Hide resolved
ql/src/test/results/clientpositive/llap/prepare_plan.q.out Show resolved Hide resolved
ql/src/test/results/clientpositive/llap/prepare_plan.q.out Show resolved Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/plan/ExprDynamicParamDesc.java Outdated Show resolved Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/plan/ExprDynamicParamDesc.java Outdated Show resolved Hide resolved
@vineetgarg02 vineetgarg02 merged commit 2544723 into apache:master Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcamachor jcamachor jcamachor approved these changes

Assignees

@vineetgarg02 vineetgarg02

Labels
tests passed
Projects
None yet
Milestone
No milestone
4 participants
@vineetgarg02 @zabetak @jcamachor @kgyrtkirk