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

Fix missing parameters in some subqueries #3539

Merged
merged 1 commit into from Jun 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions h2/src/docsrc/html/changelog.html
Expand Up @@ -21,6 +21,8 @@ <h1>Change Log</h1>

<h2>Next Version (unreleased)</h2>
<ul>
<li>Issue #3534: Subquery has incorrect empty parameters since 2.1.210 that breaks sameResultAsLast()
</li>
<li>Issue #3390: "ROW" cannot be set as a non keyword in 2.x
</li>
<li>Issue #3448: With linked table to postgreSQL, case-sensitive column names not respected in where part
Expand Down
17 changes: 6 additions & 11 deletions h2/src/main/org/h2/command/CommandContainer.java
Expand Up @@ -118,7 +118,11 @@ public CommandContainer(SessionLocal session, String sql, Prepared prepared) {

@Override
public ArrayList<? extends ParameterInterface> getParameters() {
return prepared.getParameters();
ArrayList<Parameter> parameters = prepared.getParameters();
if (parameters.size() > 0 && prepared.isWithParamValues()) {
parameters = new ArrayList<>();
}
return parameters;
}

@Override
Expand All @@ -137,20 +141,11 @@ private void recompileIfRequired() {
prepared.setModificationMetaId(0);
String sql = prepared.getSQL();
ArrayList<Token> tokens = prepared.getSQLTokens();
ArrayList<Parameter> oldParams = prepared.getParameters();
Parser parser = new Parser(session);
parser.setSuppliedParameters(prepared.getParameters());
prepared = parser.parse(sql, tokens);
long mod = prepared.getModificationMetaId();
prepared.setModificationMetaId(0);
ArrayList<Parameter> newParams = prepared.getParameters();
for (int i = 0, size = Math.min(newParams.size(), oldParams.size()); i < size; i++) {
Parameter old = oldParams.get(i);
if (old.isValueSet()) {
Value v = old.getValue(session);
Parameter p = newParams.get(i);
p.setValue(v);
}
}
prepared.prepare();
prepared.setModificationMetaId(mod);
}
Expand Down
133 changes: 74 additions & 59 deletions h2/src/main/org/h2/command/Parser.java
Expand Up @@ -449,7 +449,7 @@ public class Parser {
private Select currentSelect;
private List<TableView> cteCleanups;
private ArrayList<Parameter> parameters;
private ArrayList<Parameter> suppliedParameters;
private BitSet usedParameters = new BitSet();
private String schemaName;
private ArrayList<String> expectedList;
private boolean rightsChecked;
Expand Down Expand Up @@ -604,7 +604,6 @@ private CommandList prepareCommandList(CommandContainer command, Prepared p, Str
// Next commands may depend on results of this command.
return new CommandList(session, sql, command, list, parameters, remainingSql);
}
suppliedParameters = parameters;
try {
p = parse(remainingSql, remainingTokens);
} catch (DbException ex) {
Expand Down Expand Up @@ -669,8 +668,6 @@ Prepared parse(String sql, ArrayList<Token> tokens) {
throw e.addSQL(sql);
}
}
p.setPrepareAlways(recompileAlways);
p.setParameterList(parameters);
return p;
}

Expand All @@ -680,12 +677,12 @@ private Prepared parse(boolean withExpectedList) {
} else {
expectedList = null;
}
parameters = suppliedParameters != null ? suppliedParameters : Utils.newSmallArrayList();
currentSelect = null;
currentPrepared = null;
createView = null;
cteCleanups = null;
recompileAlways = false;
usedParameters.clear();
read();
Prepared p;
try {
Expand All @@ -697,6 +694,8 @@ private Prepared parse(boolean withExpectedList) {
}
throw t;
}
p.setPrepareAlways(recompileAlways);
p.setParameterList(parameters);
return p;
}

Expand Down Expand Up @@ -866,13 +865,6 @@ private Prepared parsePrepared() {
if (c == null) {
throw getSyntaxError();
}
if (parameters != null) {
for (int i = 0, size = parameters.size(); i < size; i++) {
if (parameters.get(i) == null) {
parameters.set(i, new Parameter(i));
}
}
}
boolean withParamValues = readIf(OPEN_BRACE);
if (withParamValues) {
do {
Expand All @@ -893,7 +885,7 @@ private Prepared parsePrepared() {
for (Parameter p : parameters) {
p.checkSet();
}
parameters.clear();
c.setWithParamValues(true);
}
if (withParamValues || c.getSQL() == null) {
setSQL(c, start);
Expand Down Expand Up @@ -1808,9 +1800,11 @@ private TableFilter readTablePrimary() {
return readCorrelation(tableFilter);
}
} else if (readIf(VALUES)) {
BitSet outerUsedParameters = initParametersScope();
TableValueConstructor query = parseValues();
alias = session.getNextSystemIdentifier(sqlCommand);
table = query.toTable(alias, null, parameters, createView != null, currentSelect);
table = query.toTable(alias, null, getUsedParameters(outerUsedParameters), createView != null,
currentSelect);
} else if (readIf(TABLE, OPEN_PAREN)) {
// Table function derived table
ArrayTableFunction function = readTableFunction(ArrayTableFunction.TABLE);
Expand Down Expand Up @@ -1898,15 +1892,17 @@ private TableFilter readCorrelation(TableFilter tableFilter) {
}

private TableFilter readDerivedTableWithCorrelation() {
BitSet outerUsedParameters = initParametersScope();
Query query = parseQueryExpression();
ArrayList<Parameter> queryParameters = getUsedParameters(outerUsedParameters);
read(CLOSE_PAREN);
Table table;
String alias;
ArrayList<String> derivedColumnNames = null;
IndexHints indexHints = null;
if (readIfUseIndex()) {
alias = session.getNextSystemIdentifier(sqlCommand);
table = query.toTable(alias, null, parameters, createView != null, currentSelect);
table = query.toTable(alias, null, queryParameters, createView != null, currentSelect);
indexHints = parseIndexHints(table);
} else {
alias = readFromAlias(null);
Expand All @@ -1919,13 +1915,13 @@ private TableFilter readDerivedTableWithCorrelation() {
derivedColumnNames.toArray(new String[0]), query, new String[1])
.toArray(new Column[0]);
}
table = query.toTable(alias, columnTemplates, parameters, createView != null, currentSelect);
table = query.toTable(alias, columnTemplates, queryParameters, createView != null, currentSelect);
if (readIfUseIndex()) {
indexHints = parseIndexHints(table);
}
} else {
alias = session.getNextSystemIdentifier(sqlCommand);
table = query.toTable(alias, null, parameters, createView != null, currentSelect);
table = query.toTable(alias, null, queryParameters, createView != null, currentSelect);
}
}
return buildTableFilter(table, alias, derivedColumnNames, indexHints);
Expand Down Expand Up @@ -2567,26 +2563,18 @@ private Explain parseExplain() {
}

private Query parseQuery() {
int paramIndex = parameters.size();
BitSet outerUsedParameters = initParametersScope();
Query command = parseQueryExpression();
int size = parameters.size();
ArrayList<Parameter> params = new ArrayList<>(size);
for (int i = paramIndex; i < size; i++) {
params.add(parameters.get(i));
}
ArrayList<Parameter> params = getUsedParameters(outerUsedParameters);
command.setParameterList(params);
command.init();
return command;
}

private Prepared parseWithStatementOrQuery(int start) {
int paramIndex = parameters.size();
BitSet outerUsedParameters = initParametersScope();
Prepared command = parseWith();
int size = parameters.size();
ArrayList<Parameter> params = new ArrayList<>(size);
for (int i = paramIndex; i < size; i++) {
params.add(parameters.get(i));
}
ArrayList<Parameter> params = getUsedParameters(outerUsedParameters);
command.setParameterList(params);
if (command instanceof Query) {
Query query = (Query) command;
Expand Down Expand Up @@ -2877,6 +2865,7 @@ private Select parseSelect(int start) {
Select command = new Select(session, currentSelect);
Select oldSelect = currentSelect;
Prepared oldPrepared = currentPrepared;
BitSet outerUsedParameters = initParametersScope();
currentSelect = command;
currentPrepared = command;
parseSelectExpressions(command);
Expand Down Expand Up @@ -2949,7 +2938,7 @@ private Select parseSelect(int start) {
command.setWindowQuery();
command.setQualify(readExpressionWithGlobalConditions());
}
command.setParameterList(parameters);
command.setParameterList(getUsedParameters(outerUsedParameters));
currentSelect = oldSelect;
currentPrepared = oldPrepared;
setSQL(command, start);
Expand Down Expand Up @@ -4882,28 +4871,30 @@ private void checkDatabaseName(String databaseName) {
}

private Parameter readParameter() {
int index = ((Token.ParameterToken) token).index();
int index = ((Token.ParameterToken) token).index() - 1;
read();
Parameter p;
if (parameters == null) {
parameters = Utils.newSmallArrayList();
}
if (index > Constants.MAX_PARAMETER_INDEX) {
throw DbException.getInvalidValueException("parameter index", index);
}
index--;
if (parameters.size() <= index) {
parameters.ensureCapacity(index + 1);
while (parameters.size() < index) {
parameters.add(null);
usedParameters.set(index);
return parameters.get(index);
}

private BitSet initParametersScope() {
BitSet outerUsedParameters = usedParameters;
usedParameters = new BitSet();
return outerUsedParameters;
}

private ArrayList<Parameter> getUsedParameters(BitSet outerUsedParameters) {
BitSet innerUsedParameters = usedParameters;
int size = innerUsedParameters.cardinality();
ArrayList<Parameter> params = new ArrayList<>(size);
if (size > 0) {
for (int i = -1; (i = innerUsedParameters.nextSetBit(i + 1)) >= 0;) {
params.add(parameters.get(i));
}
p = new Parameter(index);
parameters.add(p);
} else if ((p = parameters.get(index)) == null) {
p = new Parameter(index);
parameters.set(index, p);
}
return p;
outerUsedParameters.or(innerUsedParameters);
usedParameters = outerUsedParameters;
return params;
}

private Expression readTerm() {
Expand Down Expand Up @@ -5880,8 +5871,32 @@ private void initialize(String sql, ArrayList<Token> tokens, boolean stopOnClose
sql = "";
}
sqlCommand = sql;
this.tokens = tokens == null ? new Tokenizer(database, identifiersToUpper, identifiersToLower, nonKeywords)
.tokenize(sql, stopOnCloseParen) : tokens;
if (tokens == null) {
BitSet usedParameters = new BitSet();
this.tokens = new Tokenizer(database, identifiersToUpper, identifiersToLower, nonKeywords)
.tokenize(sql, stopOnCloseParen, usedParameters);
if (parameters == null) {
int l = usedParameters.length();
if (l > Constants.MAX_PARAMETER_INDEX) {
throw DbException.getInvalidValueException("parameter index", l);
}
if (l > 0) {
parameters = new ArrayList<>(l);
for (int i = 0; i < l; i++) {
/*
* We need to create parameters even when they aren't
* actually used, for example, VALUES ?1, ?3 needs
* parameters ?1, ?2, and ?3.
*/
parameters.add(new Parameter(i));
}
} else {
parameters = new ArrayList<>();
}
}
} else {
this.tokens = tokens;
}
resetTokenIndex();
}

Expand Down Expand Up @@ -7398,6 +7413,8 @@ private TableView parseSingleCommonTableExpression(boolean isTemporary) {
isTemporary, session, cteViewName, schema, columns, database);
List<Column> columnTemplateList;
String[] querySQLOutput = new String[1];
BitSet outerUsedParameters = initParametersScope();
ArrayList<Parameter> queryParameters;
try {
read(AS);
read(OPEN_PAREN);
Expand All @@ -7409,17 +7426,18 @@ private TableView parseSingleCommonTableExpression(boolean isTemporary) {
columnTemplateList = QueryExpressionTable.createQueryColumnTemplateList(cols, withQuery, querySQLOutput);

} finally {
queryParameters = getUsedParameters(outerUsedParameters);
TableView.destroyShadowTableForRecursiveExpression(isTemporary, session, recursiveTable);
}

return createCTEView(cteViewName,
querySQLOutput[0], columnTemplateList,
querySQLOutput[0], queryParameters, columnTemplateList,
true/* allowRecursiveQueryDetection */,
true/* add to session */,
isTemporary);
}

private TableView createCTEView(String cteViewName, String querySQL,
private TableView createCTEView(String cteViewName, String querySQL, ArrayList<Parameter> queryParameters,
List<Column> columnTemplateList, boolean allowRecursiveQueryDetection,
boolean addViewToSession, boolean isTemporary) {
Schema schema = getSchemaWithDefault();
Expand All @@ -7432,7 +7450,7 @@ private TableView createCTEView(String cteViewName, String querySQL,
TableView view;
synchronized (session) {
view = new TableView(schema, id, cteViewName, querySQL,
Copy link
Contributor

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object returned by getSchemaWithDefault(this) could be null and is dereferenced by call to TableView(...) at line 7452.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

parameters, columnTemplateArray, session,
queryParameters, columnTemplateArray, session,
allowRecursiveQueryDetection, false, true,
isTemporary);
if (!view.isRecursiveQueryDetected() && allowRecursiveQueryDetection) {
Expand All @@ -7443,7 +7461,7 @@ private TableView createCTEView(String cteViewName, String querySQL,
} else {
session.removeLocalTempTable(view);
}
view = new TableView(schema, id, cteViewName, querySQL, parameters,
view = new TableView(schema, id, cteViewName, querySQL, queryParameters,
columnTemplateArray, session,
false/* assume recursive */, false, true,
isTemporary);
Expand Down Expand Up @@ -9583,7 +9601,7 @@ public void setRightsChecked(boolean rightsChecked) {
}

public void setSuppliedParameters(ArrayList<Parameter> suppliedParameters) {
this.suppliedParameters = suppliedParameters;
this.parameters = suppliedParameters;
}

/**
Expand All @@ -9593,7 +9611,6 @@ public void setSuppliedParameters(ArrayList<Parameter> suppliedParameters) {
* @return the expression object
*/
public Expression parseExpression(String sql) {
parameters = Utils.newSmallArrayList();
initialize(sql, null, false);
read();
return readExpression();
Expand All @@ -9606,7 +9623,6 @@ public Expression parseExpression(String sql) {
* @return the expression object
*/
public Expression parseDomainConstraintExpression(String sql) {
parameters = Utils.newSmallArrayList();
initialize(sql, null, false);
read();
try {
Expand All @@ -9624,7 +9640,6 @@ public Expression parseDomainConstraintExpression(String sql) {
* @return the table object
*/
public Table parseTableName(String sql) {
parameters = Utils.newSmallArrayList();
initialize(sql, null, false);
read();
return readTableOrView();
Expand Down