dbwong commented on code in PR #1458: URL: https://github.com/apache/phoenix/pull/1458#discussion_r968907383
########## 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: If this is dead can we just throw instead? ########## phoenix-core/src/main/java/org/apache/phoenix/expression/function/StringToUUIDFunction.java: ########## @@ -0,0 +1,112 @@ +/* + * 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.expression.function; + +import java.nio.charset.Charset; +import java.sql.SQLException; +import java.util.List; +import java.util.UUID; + +import org.apache.hadoop.hbase.io.ImmutableBytesWritable; +import org.apache.phoenix.expression.Expression; +import org.apache.phoenix.parse.FunctionParseNode.Argument; +import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction; +import org.apache.phoenix.schema.SortOrder; +import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.schema.types.PBinary; +import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.types.PUUID; +import org.apache.phoenix.schema.types.PVarchar; +import org.apache.phoenix.util.UUIDUtil; + +/** + * ScalarFunction {@link ScalarFunction}. Receives a String parameter and returns a UUID. + * for example: + * SELECT STR_TO_UUID('be791257-6764-45a5-9719-cc50bf7938e4'); + * inverse function to {@link UUIDToStringFunction} + * Related to {@link UUIDRandomFunction}. + */ +@BuiltInFunction(name = StringToUUIDFunction.NAME, + args = { @Argument(allowedTypes = { PVarchar.class }) }) +public class StringToUUIDFunction extends ScalarFunction { + public static final String NAME = "STR_TO_UUID"; + + // "00000000-0000-0000-0000-000000000000".length() == 36 + private static final int UUID_TEXTUAL_LENGTH = 36; + + + public StringToUUIDFunction() { + } + + public StringToUUIDFunction(List<Expression> children) throws SQLException { + super(children); + } + + private Expression getStringExpression() { + return children.get(0); + } + + @Override + public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) { + if (!getStringExpression().evaluate(tuple, ptr)) { + return false; + } + + if (ptr.getLength() != UUID_TEXTUAL_LENGTH) { + throw new IllegalArgumentException("Invalid UUID string: " + new String(ptr.get(), + ptr.getOffset(), ptr.getLength(), Charset.forName("UTF-8"))); + } + + String value; + + // Next is done because, if not,"org.apache.phoenix.expression.function.UUIDFunctionTest", + // test "testUUIDFuntions" fails when + // "testStringToUUIDfunction(uuidStr, null, SortOrder.DESC)" + // is used (maybe stupid or bad designed test?). + // + // But I think that in real world it is not necessary. + // + // without check 'ptr.get()[ptr.getOffset() + 8] == '-'' , just only with: + // 'value = new String(ptr.get(), ptr.getOffset(), 36);' other tests works fine. + // but placed because not sure (may be removed?). + + if (ptr.get()[ptr.getOffset() + 8] == '-') { Review Comment: This type of approach seems off we should either pre-convert or known the sort order prior I feel. ########## phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java: ########## @@ -519,6 +519,74 @@ protected Random initialValue() { } }; + // Ordinal list. It makes easy to "reorder" cardinals. for instance, UUID and UUID_INDEXABLE Review Comment: "Reorder" in this case is just by line number in file in this case I think? Can you confirm? -- 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]
