You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by st...@apache.org on 2022/03/21 16:36:11 UTC
[phoenix] branch 4.x updated: PHOENIX-6665 PreparedStatement#getMetaData() fails on parametrized "select next ? values for SEQ"
This is an automated email from the ASF dual-hosted git repository.
stoty pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new a566c62 PHOENIX-6665 PreparedStatement#getMetaData() fails on parametrized "select next ? values for SEQ"
a566c62 is described below
commit a566c62fccb9e1003a8e52a7c3ad0544d3ffeac7
Author: Istvan Toth <st...@apache.org>
AuthorDate: Mon Mar 21 06:44:47 2022 +0100
PHOENIX-6665 PreparedStatement#getMetaData() fails on parametrized "select next ? values for SEQ"
---
.../phoenix/end2end/SequenceBulkAllocationIT.java | 18 ++---
.../apache/phoenix/compile/SequenceManager.java | 82 +++++++++++++---------
.../phoenix/compile/SequenceValueExpression.java | 25 +++++--
.../phoenix/jdbc/PhoenixPreparedStatement.java | 4 +-
.../java/org/apache/phoenix/schema/Sequence.java | 6 +-
5 files changed, 84 insertions(+), 51 deletions(-)
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceBulkAllocationIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceBulkAllocationIT.java
index 489513a..bac1dec 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceBulkAllocationIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SequenceBulkAllocationIT.java
@@ -1165,15 +1165,15 @@ public class SequenceBulkAllocationIT extends ParallelStatsDisabledIT {
.executeQuery(
"SELECT start_with, current_value, increment_by, cache_size, min_value, max_value, cycle_flag, sequence_schema, sequence_name FROM \"SYSTEM\".\"SEQUENCE\" where sequence_name='" + props.getNameWithoutSchema() + "'");
assertTrue(rs.next());
- assertEquals(props.startsWith, rs.getLong("start_with"));
- assertEquals(props.incrementBy, rs.getLong("increment_by"));
- assertEquals(props.cacheSize, rs.getLong("cache_size"));
- assertEquals(false, rs.getBoolean("cycle_flag"));
- assertEquals(props.getSchemaName(), rs.getString("sequence_schema"));
- assertEquals(props.getNameWithoutSchema(), rs.getString("sequence_name"));
- assertEquals(currentValue, rs.getLong("current_value"));
- assertEquals(props.minValue, rs.getLong("min_value"));
- assertEquals(props.maxValue, rs.getLong("max_value"));
+ assertEquals("start_with", props.startsWith, rs.getLong("start_with"));
+ assertEquals("increment_by", props.incrementBy, rs.getLong("increment_by"));
+ assertEquals("cache_size", props.cacheSize, rs.getLong("cache_size"));
+ assertEquals("cycle_flag", false, rs.getBoolean("cycle_flag"));
+ assertEquals("sequence_schema", props.getSchemaName(), rs.getString("sequence_schema"));
+ assertEquals("sequence_name", props.getNameWithoutSchema(), rs.getString("sequence_name"));
+ assertEquals("current_value", currentValue, rs.getLong("current_value"));
+ assertEquals("min_value", props.minValue, rs.getLong("min_value"));
+ assertEquals("max_value", props.maxValue, rs.getLong("max_value"));
assertFalse(rs.next());
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceManager.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceManager.java
index c19b3cb..0f50443 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceManager.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceManager.java
@@ -27,6 +27,7 @@ import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.expression.Expression;
+import org.apache.phoenix.expression.LiteralExpression;
import org.apache.phoenix.jdbc.PhoenixStatement;
import org.apache.phoenix.parse.ParseNode;
import org.apache.phoenix.parse.SequenceValueParseNode;
@@ -50,9 +51,9 @@ public class SequenceManager {
private int[] sequencePosition;
private List<SequenceAllocation> nextSequences;
private List<SequenceKey> currentSequences;
- private final Map<SequenceKey,SequenceValueExpression> sequenceMap = Maps.newHashMap();
+ private final Map<SequenceKey, SequenceValueExpression> sequenceMap = Maps.newHashMap();
private final BitSet isNextSequence = new BitSet();
-
+
public SequenceManager(PhoenixStatement statement) {
this.statement = statement;
}
@@ -130,64 +131,78 @@ public class SequenceManager {
int nSaltBuckets = statement.getConnection().getQueryServices().getSequenceSaltBuckets();
ParseNode numToAllocateNode = node.getNumToAllocateNode();
- long numToAllocate = determineNumToAllocate(tableName, numToAllocateNode);
+ Expression numToAllocateExp = numToAllocateExpression(tableName, numToAllocateNode);
SequenceKey key = new SequenceKey(tenantId, tableName.getSchemaName(), tableName.getTableName(), nSaltBuckets);
+
SequenceValueExpression expression = sequenceMap.get(key);
if (expression == null) {
int index = sequenceMap.size();
- expression = new SequenceValueExpression(key, node.getOp(), index, numToAllocate);
- sequenceMap.put(key, expression);
- } else if (expression.op != node.getOp() || expression.getNumToAllocate() < numToAllocate) {
- // Keep the maximum allocation size we see in a statement
+ expression = new SequenceValueExpression(key, node.getOp(), index, numToAllocateExp);
+ } else {
+ // Add the new numToAllocateExp to the expression
SequenceValueExpression oldExpression = expression;
- expression = new SequenceValueExpression(key, node.getOp(), expression.getIndex(), Math.max(expression.getNumToAllocate(), numToAllocate));
- if (oldExpression.getNumToAllocate() < numToAllocate) {
- // If we found a NEXT VALUE expression with a higher number to allocate
- // We override the original expression
- sequenceMap.put(key, expression);
- }
- }
+ expression = new SequenceValueExpression(oldExpression, node.getOp(), numToAllocateExp);
+ }
+ sequenceMap.put(key, expression);
+
// If we see a NEXT and a CURRENT, treat the CURRENT just like a NEXT
if (node.getOp() == Op.NEXT_VALUE) {
isNextSequence.set(expression.getIndex());
}
-
+
return expression;
}
+ private Expression numToAllocateExpression(TableName tableName, ParseNode numToAllocateNode) throws SQLException {
+ if (numToAllocateNode != null) {
+ final StatementContext context = new StatementContext(statement);
+ ExpressionCompiler expressionCompiler = new ExpressionCompiler(context);
+ return numToAllocateNode.accept(expressionCompiler);
+ } else {
+ // Standard Sequence Allocation Behavior
+ return LiteralExpression.newConstant(SequenceUtil.DEFAULT_NUM_SLOTS_TO_ALLOCATE);
+ }
+ }
+
/**
* If caller specified used NEXT <n> VALUES FOR <seq> expression then we have set the numToAllocate.
* If numToAllocate is > 1 we treat this as a bulk reservation of a block of sequence slots.
*
- * @throws a SQLException if we can't compile the expression
+ * @throws a SQLException if we can't evaluate the expression
*/
- private long determineNumToAllocate(TableName sequenceName, ParseNode numToAllocateNode)
+ private long determineNumToAllocate(SequenceValueExpression expression)
throws SQLException {
- if (numToAllocateNode != null) {
- final StatementContext context = new StatementContext(statement);
- ExpressionCompiler expressionCompiler = new ExpressionCompiler(context);
- Expression expression = numToAllocateNode.accept(expressionCompiler);
+ final StatementContext context = new StatementContext(statement);
+ long maxNumToAllocate = 0;
+ for (Expression numToAllocateExp : expression.getNumToAllocateExpressions()) {
ImmutableBytesWritable ptr = context.getTempPtr();
- expression.evaluate(null, ptr);
- if (ptr.getLength() == 0 || !expression.getDataType().isCoercibleTo(PLong.INSTANCE)) {
- throw SequenceUtil.getException(sequenceName.getSchemaName(), sequenceName.getTableName(), SQLExceptionCode.NUM_SEQ_TO_ALLOCATE_MUST_BE_CONSTANT);
+ numToAllocateExp.evaluate(null, ptr);
+ if (ptr.getLength() == 0 || !numToAllocateExp.getDataType().isCoercibleTo(PLong.INSTANCE)) {
+ throw SequenceUtil.getException(expression.getKey().getSchemaName(),
+ expression.getKey().getSequenceName(),
+ SQLExceptionCode.NUM_SEQ_TO_ALLOCATE_MUST_BE_CONSTANT);
}
// Parse <n> and make sure it is greater than 0. We don't support allocating 0 or negative values!
- long numToAllocate = (long) PLong.INSTANCE.toObject(ptr, expression.getDataType());
+ long numToAllocate = (long) PLong.INSTANCE.toObject(ptr, numToAllocateExp.getDataType());
if (numToAllocate < 1) {
- throw SequenceUtil.getException(sequenceName.getSchemaName(), sequenceName.getTableName(), SQLExceptionCode.NUM_SEQ_TO_ALLOCATE_MUST_BE_CONSTANT);
+ throw SequenceUtil.getException(expression.getKey().getSchemaName(),
+ expression.getKey().getSequenceName(),
+ SQLExceptionCode.NUM_SEQ_TO_ALLOCATE_MUST_BE_CONSTANT);
+ }
+ if (numToAllocate > maxNumToAllocate) {
+ maxNumToAllocate = numToAllocate;
}
- return numToAllocate;
-
- } else {
- // Standard Sequence Allocation Behavior
- return SequenceUtil.DEFAULT_NUM_SLOTS_TO_ALLOCATE;
}
+
+ return maxNumToAllocate;
}
-
+
public void validateSequences(Sequence.ValueOp action) throws SQLException {
+ if (action == Sequence.ValueOp.NOOP) {
+ return;
+ }
if (sequenceMap.isEmpty()) {
return;
}
@@ -198,7 +213,8 @@ public class SequenceManager {
currentSequences = Lists.newArrayListWithExpectedSize(maxSize);
for (Map.Entry<SequenceKey, SequenceValueExpression> entry : sequenceMap.entrySet()) {
if (isNextSequence.get(entry.getValue().getIndex())) {
- nextSequences.add(new SequenceAllocation(entry.getKey(), entry.getValue().getNumToAllocate()));
+ nextSequences.add(new SequenceAllocation(entry.getKey(),
+ determineNumToAllocate(entry.getValue())));
} else {
currentSequences.add(entry.getKey());
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceValueExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceValueExpression.java
index 71e2d02..8ba3183 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceValueExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/SequenceValueExpression.java
@@ -17,9 +17,14 @@
*/
package org.apache.phoenix.compile;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.expression.BaseTerminalExpression;
import org.apache.phoenix.expression.Determinism;
+import org.apache.phoenix.expression.Expression;
import org.apache.phoenix.expression.visitor.ExpressionVisitor;
import org.apache.phoenix.parse.SequenceValueParseNode.Op;
import org.apache.phoenix.schema.SequenceKey;
@@ -32,17 +37,25 @@ public class SequenceValueExpression extends BaseTerminalExpression {
private final SequenceKey key;
final Op op;
private final int index;
- private final long numToAllocate;
+ private final Set<Expression> numToAllocateExpressions = new HashSet<>();
- public SequenceValueExpression(SequenceKey key, Op op, int index, long numToAllocate) {
+ public SequenceValueExpression(SequenceKey key, Op op, int index, Expression numToAllocateExp) {
this.key = key;
this.op = op;
this.index = index;
- this.numToAllocate = numToAllocate;
+ this.numToAllocateExpressions.add(numToAllocateExp);
+ }
+
+ public SequenceValueExpression(SequenceValueExpression seqIn, Op op, Expression numToAllocateExp) {
+ this.key = seqIn.getKey();
+ this.op = op;
+ this.index = seqIn.getIndex();
+ this.numToAllocateExpressions.addAll(seqIn.numToAllocateExpressions);
+ this.numToAllocateExpressions.add(numToAllocateExp);
}
- public long getNumToAllocate() {
- return numToAllocate;
+ public Set<Expression> getNumToAllocateExpressions() {
+ return numToAllocateExpressions;
}
public SequenceKey getKey() {
@@ -83,7 +96,7 @@ public class SequenceValueExpression extends BaseTerminalExpression {
@Override
public String toString() {
- return op.getName() + (numToAllocate == 1 ? " VALUE " : (" " + numToAllocate + " VALUES " )) + "FOR " + SchemaUtil.getTableName(key.getSchemaName(),key.getSequenceName());
+ return op.getName() + Arrays.toString(getNumToAllocateExpressions().toArray()) + " VALUE(S) " + "FOR " + SchemaUtil.getTableName(key.getSchemaName(),key.getSequenceName());
}
@Override
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixPreparedStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixPreparedStatement.java
index 2a13121..1b2b645 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixPreparedStatement.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixPreparedStatement.java
@@ -222,7 +222,7 @@ public class PhoenixPreparedStatement extends PhoenixStatement implements Prepar
}
try {
// Just compile top level query without optimizing to get ResultSetMetaData
- QueryPlan plan = statement.compilePlan(this, Sequence.ValueOp.VALIDATE_SEQUENCE);
+ QueryPlan plan = statement.compilePlan(this, Sequence.ValueOp.NOOP);
return new PhoenixResultSetMetaData(this.getConnection(), plan.getProjector());
} finally {
int lastSetBit = 0;
@@ -245,7 +245,7 @@ public class PhoenixPreparedStatement extends PhoenixStatement implements Prepar
}
}
try {
- StatementPlan plan = statement.compilePlan(this, Sequence.ValueOp.VALIDATE_SEQUENCE);
+ StatementPlan plan = statement.compilePlan(this, Sequence.ValueOp.NOOP);
return plan.getParameterMetaData();
} finally {
int lastSetBit = 0;
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/Sequence.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/Sequence.java
index 0a9adac..13ea77d 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/Sequence.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/Sequence.java
@@ -62,7 +62,11 @@ import org.apache.phoenix.thirdparty.com.google.common.math.LongMath;
public class Sequence {
public static final int SUCCESS = 0;
- public enum ValueOp {VALIDATE_SEQUENCE, INCREMENT_SEQUENCE};
+ public enum ValueOp {
+ VALIDATE_SEQUENCE, // Check that the sequence statements are valid, during statement compilation
+ INCREMENT_SEQUENCE, // Perform the seqence operations, during execution
+ NOOP // Do not do anything, for compiling unbound prepared statements
+ };
public enum MetaOp {CREATE_SEQUENCE, DROP_SEQUENCE, RETURN_SEQUENCE};
// create empty Sequence key values used while created a sequence row