ChinmaySKulkarni commented on a change in pull request #671: PHOENIX-4845 Support using Row Value Constructors in OFFSET clause fo… URL: https://github.com/apache/phoenix/pull/671#discussion_r376563810
########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorOffsetIT.java ########## @@ -0,0 +1,943 @@ +/* + * 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.end2end; + +import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Properties; + +import org.apache.phoenix.schema.RowValueConstructorOffsetNotCoercibleException; +import org.apache.phoenix.util.PhoenixRuntime; +import org.apache.phoenix.util.PropertiesUtil; +import org.apache.phoenix.util.TestUtil; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +// RVC Based Offset - Tests +public class RowValueConstructorOffsetIT extends ParallelStatsDisabledIT { + + private static final String SIMPLE_DDL = "CREATE TABLE %s (t_id VARCHAR NOT NULL,\n" + "k1 INTEGER NOT NULL,\n" + + "k2 INTEGER NOT NULL,\n" + "v1 INTEGER,\n" + "v2 VARCHAR,\n" + + "CONSTRAINT pk PRIMARY KEY (t_id, k1, k2)) "; + + private static final String DATA_DDL = "CREATE TABLE %s (k1 TINYINT NOT NULL,\n" + "k2 TINYINT NOT NULL,\n" + + "k3 TINYINT NOT NULL,\n" + "v1 INTEGER,\n" + "CONSTRAINT pk PRIMARY KEY (k1, k2, k3)) "; + + private static final String TABLE_NAME = "T_" + generateUniqueName(); + + private static final String TABLE_ROW_KEY = "t_id, k1, k2"; + + private static final String GOOD_TABLE_ROW_KEY_VALUE = "'a', 1, 2"; + + private static final String DATA_TABLE_NAME = "T_" + generateUniqueName(); + + private static final String DATA_ROW_KEY = "k1, k2, k3"; + + private static final String GOOD_DATA_ROW_KEY_VALUE = "2, 3, 1"; + + private static final String INDEX_NAME = "INDEX_" + TABLE_NAME; + + private static final String DATA_INDEX_NAME = "INDEX_" + DATA_TABLE_NAME; + + private static final String DATA_INDEX_ROW_KEY = "k2, k1, k3"; + + private static Connection conn = null; + + @BeforeClass + public static void init() throws SQLException { + conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)); + + String dataTableDDL = String.format(DATA_DDL, DATA_TABLE_NAME); + + try (Statement statement = conn.createStatement()) { + statement.execute(dataTableDDL); + } + + try (Statement statement = conn.createStatement()) { + statement.execute(String.format(SIMPLE_DDL, TABLE_NAME)); + } + + conn.commit(); + + String upsertDML = String.format("UPSERT INTO %s VALUES(?,?,?,?)", DATA_TABLE_NAME); + + int nRows = 0; + try (PreparedStatement ps = conn.prepareStatement(upsertDML)) { + for (int k1 = 0; k1 < 4; k1++) { + ps.setInt(1, k1); + for (int k2 = 0; k2 < 4; k2++) { + ps.setInt(2, k2); + for (int k3 = 0; k3 < 4; k3++) { + ps.setInt(3, k3); + ps.setInt(4, nRows); + int result = ps.executeUpdate(); + assertEquals(1, result); + nRows++; + } + } + } + conn.commit(); + } + + String createIndex = "CREATE INDEX IF NOT EXISTS " + INDEX_NAME + " ON " + TABLE_NAME + " (k2 DESC,k1)"; + try (Statement statement = conn.createStatement()) { + statement.execute(createIndex); + } + + String createDataIndex = "CREATE INDEX IF NOT EXISTS " + DATA_INDEX_NAME + " ON " + DATA_TABLE_NAME + + " (k2 DESC,k1)"; + try (Statement statement = conn.createStatement()) { + statement.execute(createDataIndex); + } + conn.commit(); + } + + @AfterClass + public static void cleanup() { + try { + if (conn != null) { + conn.close(); + } + } catch (SQLException e) { + throw new RuntimeException(e); + } + } + + // Test RVC Offset columns must be coercible to a base table + @Test + public void testRVCOffsetNotCoercible() throws SQLException { + //'ab' is not an integer so this fails + String failureSql = String.format("SELECT %s FROM %s OFFSET (%s)=('a', 'ab', 2)", + TABLE_ROW_KEY,TABLE_NAME,TABLE_ROW_KEY); + try (Statement statement = conn.createStatement()){ + statement.execute(failureSql); + fail("Should not allow non coercible values to PK in RVC Offset"); + } catch (RowValueConstructorOffsetNotCoercibleException e) { + return; + } + } + + // Test Order By Not PK Order By Exception + @Test + public void testRVCOffsetNotAllowNonPKOrderBy() throws SQLException { + String failureSql = String.format("SELECT %s, v1 FROM %s ORDER BY v1 OFFSET (%s)=(%s)", + TABLE_ROW_KEY,TABLE_NAME,TABLE_ROW_KEY,GOOD_TABLE_ROW_KEY_VALUE); + try (Statement statement = conn.createStatement()) { + statement.execute(failureSql); + fail("Should not allow no PK order by with RVC Offset"); + } catch (RowValueConstructorOffsetNotCoercibleException e) { + return; + } + + } + + // Test Order By Partial PK Order By Exception + @Test + public void testRVCOffsetNotAllowPartialPKOrderBy() throws SQLException { + String failureSql = String.format("SELECT %s FROM %s ORDER BY k1 OFFSET (%s)=(%s)", + TABLE_ROW_KEY,TABLE_NAME, TABLE_ROW_KEY, GOOD_TABLE_ROW_KEY_VALUE); + try (Statement statement = conn.createStatement()){ + statement.execute(failureSql); + fail("Should not allow partial PK order by with RVC Offset"); + } catch (RowValueConstructorOffsetNotCoercibleException e) { + return; + } + } + + // Test Order By Different Sort PK Order By Exception + @Test + public void testRVCOffsetSamePKDifferentSortOrderBy() throws SQLException { Review comment: Just wondering whether it is necessary to also have a test where the table itself has a mix of descending/ascending pk. The current test has all ascending pk. I'm open to excluding such a test if you think it is not required @dbwong ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
