alejandro-anadon commented on code in PR #1458: URL: https://github.com/apache/phoenix/pull/1458#discussion_r969388384
########## phoenix-core/src/it/java/org/apache/phoenix/end2end/UUIDTypeIT.java: ########## @@ -0,0 +1,231 @@ +/* + * 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.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; + +import static org.junit.Assert.assertTrue; + +import java.sql.Array; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.util.Collection; +import java.util.UUID; + +import org.apache.phoenix.jdbc.PhoenixPreparedStatement; +import org.apache.phoenix.jdbc.PhoenixResultSet; +import org.apache.phoenix.util.SchemaUtil; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runners.Parameterized.Parameters; + +@Category(ParallelStatsDisabledTest.class) +public class UUIDTypeIT extends BaseQueryIT { + + protected static String SCHEMA1 = "SCHEMAFORUUIDTEST"; + + public UUIDTypeIT(String indexDDL, boolean columnEncoded, boolean keepDeletedCells) { + super(indexDDL, columnEncoded, keepDeletedCells); + } + + @Parameters(name="UUIDTypeIT_{index}") // name is used by failsafe as file name in reports + public static synchronized Collection<Object> data() { + return BaseQueryIT.allIndexes(); + } + + @Test + public void testSimple() throws Exception { + internalTestSimple(false, false); // No DESC in pk, neither in INDEX + internalTestSimple(false, true); // No DESC in pk , DESC in INDEX + internalTestSimple(true, false); // DESC in pk, No DESC in INDEX + internalTestSimple(true, true); // DESC in pk, also in INDEX + } + + public void internalTestSimple(boolean descInPk, boolean descInIndex) throws Exception { + String baseTable = SchemaUtil.getTableName(SCHEMA1, generateUniqueName()); + + UUID uuidRecord1PK = UUID.randomUUID(); + UUID uuidRecord1UUID = UUID.randomUUID(); + + UUID uuidRecord2PK = UUID.randomUUID(); + UUID uuidRecord2UUID = UUID.randomUUID(); + UUID uuidRecord2Array[]=new UUID[7]; + + + for(int x=0;x<uuidRecord2Array.length;x++) + uuidRecord2Array[x]=UUID.randomUUID(); + + + + UUID uuidRecord3PK = UUID.randomUUID(); + + try (Connection conn = DriverManager.getConnection(getUrl())) { + String baseTableDDL ="CREATE TABLE " + baseTable+ " (UUID_PK UUID NOT NULL, " + + "UUID_NOT_PK UUID , " + + "UUID_ARRAY UUID ARRAY , " + + "CONSTRAINT NAME_PK PRIMARY KEY(UUID_PK"+(descInPk?" DESC":"")+"))"; + conn.createStatement().execute(baseTableDDL); + + String upsert = "UPSERT INTO " + baseTable + "(UUID_PK , UUID_NOT_PK , UUID_ARRAY ) VALUES (?,?,?)"; + + PreparedStatement ps = conn.prepareStatement(upsert); + if (ps instanceof PhoenixPreparedStatement) { + PhoenixPreparedStatement phops = (PhoenixPreparedStatement) ps; + phops.setUUID(1, uuidRecord1PK); + phops.setUUID(2, uuidRecord1UUID); + } else { + //This should be dead code + ps.setObject(1, uuidRecord1PK); Review Comment: I intentionally left it out in doubt as to whether or not it is acceptable to add the 'public void setUUID(int parameterIndex, UUID o)' method to the 'org.apache.phoenix.jdbc.PhoenixPreparedStatement.java' class (just like the ' public UUID getUUID(int columnIndex)' from class 'org.apache.phoenix.jdbc.PhoenixResultSet.java.java). I was doubting if it is right or not to incorporate them. From the point of view of use, I see them as useful; on the other hand, as they are not in the standard java.sql.PreparedStatement and import java.sql.ResultSet interfaces, the standard is left; and that is usually not good. Therefore, depending on whether or not the addition of methods is accepted, either the 'if' or the 'else' is left over. I return the question. Are they accepted or not? Depending on what you tell me, I leave in the test either the 'if' or the 'else' and so there will not be dead code. in the test it is right now as if it had been accepted; being the 'else' dead code that can be removed. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
