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.

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

refactor: use analyze to get statement params #478

Merged
merged 18 commits into from Nov 25, 2022

Conversation

olavloite
Copy link
Collaborator

@olavloite olavloite commented Nov 11, 2022

Execute statements in PLAN mode to get parameter metadata instead of re-writing the SQL statements to a SELECT.

Fixes #460

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #478 (fcd05a3) into postgresql-dialect (ef49d74) will increase coverage by 0.11%.
The diff coverage is 97.07%.

@@                   Coverage Diff                    @@
##             postgresql-dialect     #478      +/-   ##
========================================================
+ Coverage                 87.65%   87.77%   +0.11%     
+ Complexity                 2048     2033      -15     
========================================================
  Files                       123      121       -2     
  Lines                      6799     6732      -67     
  Branches                    972      946      -26     
========================================================
- Hits                       5960     5909      -51     
+ Misses                      589      585       -4     
+ Partials                    250      238      -12     
Flag Coverage Δ
all_tests 87.77% <97.07%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...panner/pgadapter/wireprotocol/DescribeMessage.java 84.05% <85.71%> (-2.06%) ⬇️
...pter/statements/IntermediatePreparedStatement.java 90.90% <89.79%> (+3.75%) ⬆️
...gle/cloud/spanner/pgadapter/ConnectionHandler.java 86.60% <100.00%> (ø)
...oud/spanner/pgadapter/metadata/DescribeResult.java 100.00% <100.00%> (ø)
...google/cloud/spanner/pgadapter/parsers/Parser.java 86.36% <100.00%> (ø)
...panner/pgadapter/statements/BackendConnection.java 92.59% <100.00%> (+0.49%) ⬆️
...ud/spanner/pgadapter/statements/CopyStatement.java 94.81% <100.00%> (+0.03%) ⬆️
.../spanner/pgadapter/statements/CopyToStatement.java 99.06% <100.00%> (+0.01%) ⬆️
...nner/pgadapter/statements/DeallocateStatement.java 97.36% <100.00%> (+0.14%) ⬆️
...spanner/pgadapter/statements/ExecuteStatement.java 91.52% <100.00%> (+0.29%) ⬆️
... and 12 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Disabling clirr as it only adds noise, and somehow refuses to pick up
the latest ignore.
@olavloite olavloite changed the title refactor: [WIP] use analyze to get statement params refactor: use analyze to get statement params Nov 18, 2022
@olavloite olavloite marked this pull request as ready for review November 18, 2022 17:30
int max = 0;
for (int i = 0; i < parameters.getFieldsCount(); i++) {
try {
int paramIndex = Integer.parseInt(parameters.getFields(i).getName().substring(1));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this required? What are the extra fields that are coming?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameters are all named p1, p2, ... p10, ...

So we need to parse the parameter index from the name by skipping the p, and we cannot use the position in the list as the index, as the order is textual, meaning that they are actually returned as (p1, p10, p2, ...)

@olavloite olavloite merged commit 76c14c5 into postgresql-dialect Nov 25, 2022
@olavloite olavloite deleted the analyze-params branch November 25, 2022 15:05
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.

Preparing an invalid statement during a read/write transaction does not invalidate the transaction
2 participants