[
https://issues.apache.org/jira/browse/PHOENIX-7263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17824628#comment-17824628
]
ASF GitHub Bot commented on PHOENIX-7263:
-----------------------------------------
stoty commented on code in PR #1850:
URL: https://github.com/apache/phoenix/pull/1850#discussion_r1517205871
##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/SplitKeyUtil.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.compile;
+
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.exception.SQLExceptionInfo;
+import org.apache.phoenix.expression.Expression;
+import org.apache.phoenix.parse.BindParseNode;
+import org.apache.phoenix.parse.ParseNode;
+import org.apache.phoenix.schema.PDatum;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PDataType;
+import org.apache.phoenix.schema.types.PVarbinary;
+import org.apache.phoenix.util.ByteUtil;
+
+import java.sql.SQLException;
+import java.util.List;
+
+public class SplitKeyUtil {
Review Comment:
Do we need a new Util class ?
Do we have an existing Util class that would make sense for this ?
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseIndexIT.java:
##########
@@ -134,6 +134,51 @@ protected BaseIndexIT(boolean localIndex, boolean
uncovered, boolean mutable, St
this.tableDDLOptions = optionBuilder.toString();
}
+ @Test
+ public void testCreateIndexWithRowValueConstructorSplitKeys() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String tableName = "TBL_" + generateUniqueName();
+ String indexName = "IND_" + generateUniqueName();
+ String fullTableName =
SchemaUtil.getTableName(TestUtil.DEFAULT_SCHEMA_NAME, tableName);
+ String fullIndexName =
SchemaUtil.getTableName(TestUtil.DEFAULT_SCHEMA_NAME, indexName);
+
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ conn.setAutoCommit(false);
+ String splitKeys = "('foo', 'a', 1, 100, 1.0,
TO_DATE('2024-01-01')), " +
+ "('john', 'b', 2, 101, 2.0, TO_DATE('2024-01-01')), " +
+ "('tom', 'c', 3, 102, 3.0, TO_DATE('2024-01-01'))";
+ String ddl ="CREATE TABLE " + fullTableName +
TestUtil.TEST_TABLE_SCHEMA + tableDDLOptions + " SPLIT ON (" +
+ splitKeys + ")";
+ Statement stmt = conn.createStatement();
+ stmt.execute(ddl);
+ BaseTest.populateTestTable(fullTableName);
+ String indexSplitKeys = "('a',111, 'foo', 'a', 1, 100, 1.0,
TO_DATE('2024-01-01')), " +
+ "('e','222','john', 'b', 2, 101, 2.0,
TO_DATE('2024-01-01')), " +
+ "('i', '333', 'tom', 'c', 3, 102, 3.0,
TO_DATE('2024-01-01'))";
+ ddl = "CREATE " + (localIndex ? "LOCAL" : "") + (uncovered ?
"UNCOVERED" : "")
+ + " INDEX " + indexName + " ON " + fullTableName
+ + " (char_col1 ASC, int_col1 ASC)"
+ + (uncovered ? "" : " INCLUDE (long_col1, long_col2)") +
(localIndex ? "" :
+ " SPLIT ON (" + indexSplitKeys + ")");
+ stmt.execute(ddl);
+
+ String query = "SELECT d.char_col1, int_col1 from " +
fullTableName + " as d";
+ ResultSet rs = conn.createStatement().executeQuery(query);
+ assertTrue(rs.next());
+ assertEquals("chara", rs.getString(1));
+ assertEquals("chara", rs.getString("char_col1"));
+ assertEquals(2, rs.getInt(2));
+ assertTrue(rs.next());
+ assertEquals("chara", rs.getString(1));
+ assertEquals(3, rs.getInt(2));
+ assertTrue(rs.next());
+ assertEquals("chara", rs.getString(1));
+ assertEquals(4, rs.getInt(2));
+ assertFalse(rs.next());
+ conn.createStatement().execute("DROP INDEX " + indexName + " ON "
+ fullTableName);
Review Comment:
Nit: We already have a stmt object. We could also create the stmt in the
try-with-reources block.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/BaseIndexIT.java:
##########
@@ -134,6 +134,51 @@ protected BaseIndexIT(boolean localIndex, boolean
uncovered, boolean mutable, St
this.tableDDLOptions = optionBuilder.toString();
}
+ @Test
+ public void testCreateIndexWithRowValueConstructorSplitKeys() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String tableName = "TBL_" + generateUniqueName();
+ String indexName = "IND_" + generateUniqueName();
+ String fullTableName =
SchemaUtil.getTableName(TestUtil.DEFAULT_SCHEMA_NAME, tableName);
+ String fullIndexName =
SchemaUtil.getTableName(TestUtil.DEFAULT_SCHEMA_NAME, indexName);
+
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ conn.setAutoCommit(false);
+ String splitKeys = "('foo', 'a', 1, 100, 1.0,
TO_DATE('2024-01-01')), " +
+ "('john', 'b', 2, 101, 2.0, TO_DATE('2024-01-01')), " +
+ "('tom', 'c', 3, 102, 3.0, TO_DATE('2024-01-01'))";
+ String ddl ="CREATE TABLE " + fullTableName +
TestUtil.TEST_TABLE_SCHEMA + tableDDLOptions + " SPLIT ON (" +
+ splitKeys + ")";
+ Statement stmt = conn.createStatement();
+ stmt.execute(ddl);
+ BaseTest.populateTestTable(fullTableName);
+ String indexSplitKeys = "('a',111, 'foo', 'a', 1, 100, 1.0,
TO_DATE('2024-01-01')), " +
+ "('e','222','john', 'b', 2, 101, 2.0,
TO_DATE('2024-01-01')), " +
+ "('i', '333', 'tom', 'c', 3, 102, 3.0,
TO_DATE('2024-01-01'))";
+ ddl = "CREATE " + (localIndex ? "LOCAL" : "") + (uncovered ?
"UNCOVERED" : "")
+ + " INDEX " + indexName + " ON " + fullTableName
+ + " (char_col1 ASC, int_col1 ASC)"
+ + (uncovered ? "" : " INCLUDE (long_col1, long_col2)") +
(localIndex ? "" :
+ " SPLIT ON (" + indexSplitKeys + ")");
+ stmt.execute(ddl);
+
Review Comment:
We could check the regions and start keys explicitly.
I think that there is already test code for that and we'd just need to use /
copy that.
> Row value constructor split keys not allowed on indexes
> -------------------------------------------------------
>
> Key: PHOENIX-7263
> URL: https://issues.apache.org/jira/browse/PHOENIX-7263
> Project: Phoenix
> Issue Type: Bug
> Reporter: Rajeshbabu Chintaguntla
> Assignee: Rajeshbabu Chintaguntla
> Priority: Major
> Fix For: 5.2.0, 5.3.0, 5.1.4
>
>
> While creating indexes if we pass row value constructor split keys getting
> following error same is passing with create table because while creating the
> table properly building the split keys using expression compiler which is not
> the case with index creation.
> {noformat}
> java.lang.ClassCastException:
> org.apache.phoenix.expression.RowValueConstructorExpression cannot be cast to
> org.apache.phoenix.expression.LiteralExpression
> at
> org.apache.phoenix.compile.CreateIndexCompiler.compile(CreateIndexCompiler.java:77)
> at
> org.apache.phoenix.jdbc.PhoenixStatement$ExecutableCreateIndexStatement.compilePlan(PhoenixStatement.java:1205)
> at
> org.apache.phoenix.jdbc.PhoenixStatement$ExecutableCreateIndexStatement.compilePlan(PhoenixStatement.java:1191)
> at
> org.apache.phoenix.jdbc.PhoenixStatement$2.call(PhoenixStatement.java:435)
> at
> org.apache.phoenix.jdbc.PhoenixStatement$2.call(PhoenixStatement.java:425)
> at org.apache.phoenix.call.CallRunner.run(CallRunner.java:53)
> at
> org.apache.phoenix.jdbc.PhoenixStatement.executeMutation(PhoenixStatement.java:424)
> at
> org.apache.phoenix.jdbc.PhoenixStatement.executeMutation(PhoenixStatement.java:412)
> at
> org.apache.phoenix.jdbc.PhoenixStatement.execute(PhoenixStatement.java:2009)
> at sqlline.Commands.executeSingleQuery(Commands.java:1054)
> at sqlline.Commands.execute(Commands.java:1003)
> at sqlline.Commands.sql(Commands.java:967)
> at sqlline.SqlLine.dispatch(SqlLine.java:734)
> at sqlline.SqlLine.begin(SqlLine.java:541)
> at sqlline.SqlLine.start(SqlLine.java:267)
> at sqlline.SqlLine.main(SqlLine.java:206)
> {noformat}
> In create table:
> {code:java}
> final byte[][] splits = new byte[splitNodes.size()][];
> ImmutableBytesWritable ptr = context.getTempPtr();
> ExpressionCompiler expressionCompiler = new
> ExpressionCompiler(context);
> for (int i = 0; i < splits.length; i++) {
> ParseNode node = splitNodes.get(i);
> if (node instanceof BindParseNode) {
> context.getBindManager().addParamMetaData((BindParseNode)
> node, VARBINARY_DATUM);
> }
> if (node.isStateless()) {
> Expression expression = node.accept(expressionCompiler);
> if (expression.evaluate(null, ptr)) {;
> splits[i] = ByteUtil.copyKeyBytesIfNecessary(ptr);
> continue;
> }
> }
> throw new
> SQLExceptionInfo.Builder(SQLExceptionCode.SPLIT_POINT_NOT_CONSTANT)
> .setMessage("Node: " + node).build().buildException();
> }
> {code}
> Where as in indexing expecting only literals.
> {code:java}
> final byte[][] splits = new byte[splitNodes.size()][];
> for (int i = 0; i < splits.length; i++) {
> ParseNode node = splitNodes.get(i);
> if (!node.isStateless()) {
> throw new
> SQLExceptionInfo.Builder(SQLExceptionCode.SPLIT_POINT_NOT_CONSTANT)
> .setMessage("Node: " + node).build().buildException();
> }
> LiteralExpression expression =
> (LiteralExpression)node.accept(expressionCompiler);
> splits[i] = expression.getBytes();
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
