From ceaaecca73014438bcb3b959240c895a6df7fdf2 Mon Sep 17 00:00:00 2001 From: Ramkrishna Date: Mon, 27 Jan 2014 17:21:09 +0530 Subject: [PATCH 1/3] Review comments fix --- phoenix-core/src/main/antlr3/PhoenixSQL.g | 18 ++- .../org/apache/phoenix/parse/ColumnDef.java | 139 ++++++++++-------- .../org/apache/phoenix/schema/PDataType.java | 26 +--- .../org/apache/phoenix/end2end/ArrayTest.java | 96 +++++------- 4 files changed, 133 insertions(+), 146 deletions(-) diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g index 5dcfe813..26b82701 100644 --- a/phoenix-core/src/main/antlr3/PhoenixSQL.g +++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g @@ -521,11 +521,19 @@ dyn_column_def returns [ColumnDef ret] dyn_column_name_or_def returns [ColumnDef ret] : c=column_name (dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? )? (lsq=LSQUARE (a=NUMBER)? RSQUARE)? - {$ret = factory.columnDef(c, dt, true, - l == null ? null : Integer.parseInt( l.getText() ), - s == null ? null : Integer.parseInt( s.getText() ), - false, - null); } + { + if(lsq != null) + { + throwRecognitionException(lsq); + } else + { + $ret = factory.columnDef(c, dt, true, + l == null ? null : Integer.parseInt( l.getText() ), + s == null ? null : Integer.parseInt( s.getText() ), + false, + null); + } + } ; select_expression returns [SelectStatement ret] diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java index fb1b8702..fbeef333 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java @@ -69,64 +69,87 @@ public class ColumnDef { this.arrSize = 0; } this.isNull = isNull; - if (this.dataType == PDataType.CHAR) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.VARCHAR) { - if (maxLength != null && maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.DECIMAL) { - Integer origMaxLength = maxLength; - maxLength = maxLength == null ? PDataType.MAX_PRECISION : maxLength; - // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; - if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - // When a precision is specified and a scale is not specified, it is set to 0. - // - // This is the standard as specified in - // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 - // and - // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. - // Otherwise, if scale is bigger than maxLength, just set it to the maxLength; - // - // When neither a precision nor a scale is specified, the precision and scale is - // ignored. All decimal are stored with as much decimal points as possible. - scale = scale == null ? - origMaxLength == null ? null : PDataType.DEFAULT_SCALE : - scale > maxLength ? maxLength : scale; - } else if (this.dataType == PDataType.BINARY) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.INTEGER) { - maxLength = PDataType.INT_PRECISION; - scale = PDataType.ZERO; - } else if (this.dataType == PDataType.LONG) { - maxLength = PDataType.LONG_PRECISION; - scale = PDataType.ZERO; - } else { - // ignore maxLength and scale for other types. - maxLength = null; - scale = null; - } + if (!this.isArray) { + if (this.dataType == PDataType.CHAR) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.MISSING_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.VARCHAR) { + if (maxLength != null && maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.DECIMAL) { + Integer origMaxLength = maxLength; + maxLength = maxLength == null ? PDataType.MAX_PRECISION + : maxLength; + // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; + if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + // When a precision is specified and a scale is not + // specified, it is set to 0. + // + // This is the standard as specified in + // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 + // and + // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. + // Otherwise, if scale is bigger than maxLength, just set it + // to the maxLength; + // + // When neither a precision nor a scale is specified, the + // precision and scale is + // ignored. All decimal are stored with as much decimal + // points as possible. + scale = scale == null ? origMaxLength == null ? null + : PDataType.DEFAULT_SCALE + : scale > maxLength ? maxLength : scale; + } else if (this.dataType == PDataType.BINARY) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.MISSING_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.INTEGER) { + maxLength = PDataType.INT_PRECISION; + scale = PDataType.ZERO; + } else if (this.dataType == PDataType.LONG) { + maxLength = PDataType.LONG_PRECISION; + scale = PDataType.ZERO; + } else { + // ignore maxLength and scale for other types. + maxLength = null; + scale = null; + } + } else { + // ignore maxLength and scale for other types. + maxLength = null; + scale = null; + } this.maxLength = maxLength; this.scale = scale; this.isPK = isPK; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java index 8d14fb0c..850ae136 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java @@ -19,32 +19,21 @@ */ package org.apache.phoenix.schema; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.math.MathContext; -import java.math.RoundingMode; -import java.sql.Date; -import java.sql.Time; -import java.sql.Timestamp; -import java.sql.Types; +import java.math.*; +import java.sql.*; import java.text.Format; import java.util.Map; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Base64; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.phoenix.query.KeyRange; +import org.apache.phoenix.query.QueryConstants; +import org.apache.phoenix.util.*; import com.google.common.collect.ImmutableMap; import com.google.common.math.LongMath; -import com.google.common.primitives.Booleans; -import com.google.common.primitives.Doubles; -import com.google.common.primitives.Longs; -import org.apache.phoenix.query.KeyRange; -import org.apache.phoenix.query.QueryConstants; -import org.apache.phoenix.util.ByteUtil; -import org.apache.phoenix.util.DateUtil; -import org.apache.phoenix.util.NumberUtil; -import org.apache.phoenix.util.StringUtil; +import com.google.common.primitives.*; /** @@ -5193,9 +5182,6 @@ public Integer estimateByteSizeFromLength(Integer length) { if (isFixedWidth()) { return getByteSize(); } - if(isArrayType()) { - return null; - } // If not fixed width, default to say the byte size is the same as length. return length; } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java index e39e576d..766a224f 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java @@ -19,33 +19,20 @@ */ package org.apache.phoenix.end2end; -import static org.apache.phoenix.util.TestUtil.B_VALUE; -import static org.apache.phoenix.util.TestUtil.PHOENIX_JDBC_URL; -import static org.apache.phoenix.util.TestUtil.ROW1; -import static org.apache.phoenix.util.TestUtil.TABLE_WITH_ARRAY; -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.assertNull; -import static org.junit.Assert.assertTrue; +import static org.apache.phoenix.util.TestUtil.*; +import static org.junit.Assert.*; -import java.sql.Array; -import java.sql.Connection; -import java.sql.Date; -import java.sql.DriverManager; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; +import java.sql.*; import java.util.Properties; +import org.apache.phoenix.exception.PhoenixParserException; +import org.apache.phoenix.query.BaseTest; +import org.apache.phoenix.schema.PhoenixArray; +import org.apache.phoenix.util.PhoenixRuntime; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import com.google.common.primitives.Floats; -import org.apache.phoenix.query.BaseTest; -import org.apache.phoenix.schema.PhoenixArray; -import org.apache.phoenix.util.PhoenixRuntime; public class ArrayTest extends BaseClientManagedTimeTest { @@ -437,50 +424,33 @@ public void testUpsertSelectWithColumnRef() throws Exception { } } - @Ignore //TODO: Ram to fix @Test - public void testUpsertSelectWithSelectAsSubQuery3() throws Exception { - long ts = nextTimestamp(); - String tenantId = getOrganizationId(); - createTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - //initTablesWithArrays(tenantId, null, ts, false); - try { - createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - initSimpleArrayTable(tenantId, null, ts, false); - Properties props = new Properties(TEST_PROPERTIES); - props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, - Long.toString(ts + 2)); // Execute at timestamp 2 - Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, - props); - // TODO: this is invalid, as you can't have an array reference in upsert - String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" - + tenantId + "','00A123122312312',2.0d)"; - PreparedStatement statement = conn.prepareStatement(query); - int executeUpdate = statement.executeUpdate(); - assertEquals(1, executeUpdate); - conn.commit(); - statement.close(); - conn.close(); - // create another connection - props = new Properties(TEST_PROPERTIES); - props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, - Long.toString(ts + 4)); - conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); - query = "SELECT ARRAY_ELEM(a_double_array,3) FROM table_with_array"; - statement = conn.prepareStatement(query); - ResultSet rs = statement.executeQuery(); - assertTrue(rs.next()); - // Need to support primitive - Double[] doubleArr = new Double[1]; - doubleArr[0] = 2.0d; - conn.createArrayOf("DOUBLE", doubleArr); - Double result = rs.getDouble(1); - assertEquals(result, doubleArr[0]); - - } finally { - } + // TODO : currently not supported + public void testUpsertSelectWithSpecificIndexOfAnArray() throws Exception { + long ts = nextTimestamp(); + String tenantId = getOrganizationId(); + createTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + // initTablesWithArrays(tenantId, null, ts, false); + Connection conn = null; + try { + createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + initSimpleArrayTable(tenantId, null, ts, false); + Properties props = new Properties(TEST_PROPERTIES); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, + Long.toString(ts + 2)); // Execute at timestamp 2 + conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); + // TODO: this is invalid, as you can't have an array reference in + // upsert + String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" + + tenantId + "','00A123122312312',2.0d)"; + PreparedStatement statement = conn.prepareStatement(query); + fail("Should have failed with parser Exception"); + } catch (PhoenixParserException e) { + // Correct + } finally { + } } @Test From 8eeb0e5fbbf5d2aa7daa3a7932c69e46717c70f7 Mon Sep 17 00:00:00 2001 From: Ramkrishna Date: Mon, 27 Jan 2014 17:22:52 +0530 Subject: [PATCH 2/3] Revert "Review comments fix" This reverts commit ceaaecca73014438bcb3b959240c895a6df7fdf2. --- phoenix-core/src/main/antlr3/PhoenixSQL.g | 18 +-- .../org/apache/phoenix/parse/ColumnDef.java | 139 ++++++++---------- .../org/apache/phoenix/schema/PDataType.java | 26 +++- .../org/apache/phoenix/end2end/ArrayTest.java | 96 +++++++----- 4 files changed, 146 insertions(+), 133 deletions(-) diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g index 26b82701..5dcfe813 100644 --- a/phoenix-core/src/main/antlr3/PhoenixSQL.g +++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g @@ -521,19 +521,11 @@ dyn_column_def returns [ColumnDef ret] dyn_column_name_or_def returns [ColumnDef ret] : c=column_name (dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? )? (lsq=LSQUARE (a=NUMBER)? RSQUARE)? - { - if(lsq != null) - { - throwRecognitionException(lsq); - } else - { - $ret = factory.columnDef(c, dt, true, - l == null ? null : Integer.parseInt( l.getText() ), - s == null ? null : Integer.parseInt( s.getText() ), - false, - null); - } - } + {$ret = factory.columnDef(c, dt, true, + l == null ? null : Integer.parseInt( l.getText() ), + s == null ? null : Integer.parseInt( s.getText() ), + false, + null); } ; select_expression returns [SelectStatement ret] diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java index fbeef333..fb1b8702 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java @@ -69,87 +69,64 @@ public class ColumnDef { this.arrSize = 0; } this.isNull = isNull; - if (!this.isArray) { - if (this.dataType == PDataType.CHAR) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.MISSING_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.VARCHAR) { - if (maxLength != null && maxLength < 1) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.DECIMAL) { - Integer origMaxLength = maxLength; - maxLength = maxLength == null ? PDataType.MAX_PRECISION - : maxLength; - // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; - if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - // When a precision is specified and a scale is not - // specified, it is set to 0. - // - // This is the standard as specified in - // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 - // and - // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. - // Otherwise, if scale is bigger than maxLength, just set it - // to the maxLength; - // - // When neither a precision nor a scale is specified, the - // precision and scale is - // ignored. All decimal are stored with as much decimal - // points as possible. - scale = scale == null ? origMaxLength == null ? null - : PDataType.DEFAULT_SCALE - : scale > maxLength ? maxLength : scale; - } else if (this.dataType == PDataType.BINARY) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.MISSING_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder( - SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()) - .build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.INTEGER) { - maxLength = PDataType.INT_PRECISION; - scale = PDataType.ZERO; - } else if (this.dataType == PDataType.LONG) { - maxLength = PDataType.LONG_PRECISION; - scale = PDataType.ZERO; - } else { - // ignore maxLength and scale for other types. - maxLength = null; - scale = null; - } - } else { - // ignore maxLength and scale for other types. - maxLength = null; - scale = null; - } + if (this.dataType == PDataType.CHAR) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.VARCHAR) { + if (maxLength != null && maxLength < 1) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.DECIMAL) { + Integer origMaxLength = maxLength; + maxLength = maxLength == null ? PDataType.MAX_PRECISION : maxLength; + // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; + if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + // When a precision is specified and a scale is not specified, it is set to 0. + // + // This is the standard as specified in + // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 + // and + // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. + // Otherwise, if scale is bigger than maxLength, just set it to the maxLength; + // + // When neither a precision nor a scale is specified, the precision and scale is + // ignored. All decimal are stored with as much decimal points as possible. + scale = scale == null ? + origMaxLength == null ? null : PDataType.DEFAULT_SCALE : + scale > maxLength ? maxLength : scale; + } else if (this.dataType == PDataType.BINARY) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()).build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.INTEGER) { + maxLength = PDataType.INT_PRECISION; + scale = PDataType.ZERO; + } else if (this.dataType == PDataType.LONG) { + maxLength = PDataType.LONG_PRECISION; + scale = PDataType.ZERO; + } else { + // ignore maxLength and scale for other types. + maxLength = null; + scale = null; + } this.maxLength = maxLength; this.scale = scale; this.isPK = isPK; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java index 850ae136..8d14fb0c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java @@ -19,21 +19,32 @@ */ package org.apache.phoenix.schema; -import java.math.*; -import java.sql.*; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.math.MathContext; +import java.math.RoundingMode; +import java.sql.Date; +import java.sql.Time; +import java.sql.Timestamp; +import java.sql.Types; import java.text.Format; import java.util.Map; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Base64; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.phoenix.query.KeyRange; -import org.apache.phoenix.query.QueryConstants; -import org.apache.phoenix.util.*; import com.google.common.collect.ImmutableMap; import com.google.common.math.LongMath; -import com.google.common.primitives.*; +import com.google.common.primitives.Booleans; +import com.google.common.primitives.Doubles; +import com.google.common.primitives.Longs; +import org.apache.phoenix.query.KeyRange; +import org.apache.phoenix.query.QueryConstants; +import org.apache.phoenix.util.ByteUtil; +import org.apache.phoenix.util.DateUtil; +import org.apache.phoenix.util.NumberUtil; +import org.apache.phoenix.util.StringUtil; /** @@ -5182,6 +5193,9 @@ public Integer estimateByteSizeFromLength(Integer length) { if (isFixedWidth()) { return getByteSize(); } + if(isArrayType()) { + return null; + } // If not fixed width, default to say the byte size is the same as length. return length; } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java index 766a224f..e39e576d 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java @@ -19,20 +19,33 @@ */ package org.apache.phoenix.end2end; -import static org.apache.phoenix.util.TestUtil.*; -import static org.junit.Assert.*; +import static org.apache.phoenix.util.TestUtil.B_VALUE; +import static org.apache.phoenix.util.TestUtil.PHOENIX_JDBC_URL; +import static org.apache.phoenix.util.TestUtil.ROW1; +import static org.apache.phoenix.util.TestUtil.TABLE_WITH_ARRAY; +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.assertNull; +import static org.junit.Assert.assertTrue; -import java.sql.*; +import java.sql.Array; +import java.sql.Connection; +import java.sql.Date; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.Properties; -import org.apache.phoenix.exception.PhoenixParserException; -import org.apache.phoenix.query.BaseTest; -import org.apache.phoenix.schema.PhoenixArray; -import org.apache.phoenix.util.PhoenixRuntime; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import com.google.common.primitives.Floats; +import org.apache.phoenix.query.BaseTest; +import org.apache.phoenix.schema.PhoenixArray; +import org.apache.phoenix.util.PhoenixRuntime; public class ArrayTest extends BaseClientManagedTimeTest { @@ -424,33 +437,50 @@ public void testUpsertSelectWithColumnRef() throws Exception { } } + @Ignore //TODO: Ram to fix @Test - // TODO : currently not supported - public void testUpsertSelectWithSpecificIndexOfAnArray() throws Exception { - long ts = nextTimestamp(); - String tenantId = getOrganizationId(); - createTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - // initTablesWithArrays(tenantId, null, ts, false); - Connection conn = null; - try { - createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - initSimpleArrayTable(tenantId, null, ts, false); - Properties props = new Properties(TEST_PROPERTIES); - props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, - Long.toString(ts + 2)); // Execute at timestamp 2 - conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); - // TODO: this is invalid, as you can't have an array reference in - // upsert - String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" - + tenantId + "','00A123122312312',2.0d)"; - PreparedStatement statement = conn.prepareStatement(query); - fail("Should have failed with parser Exception"); - } catch (PhoenixParserException e) { - // Correct - } finally { - } + public void testUpsertSelectWithSelectAsSubQuery3() throws Exception { + long ts = nextTimestamp(); + String tenantId = getOrganizationId(); + createTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + //initTablesWithArrays(tenantId, null, ts, false); + try { + createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + initSimpleArrayTable(tenantId, null, ts, false); + Properties props = new Properties(TEST_PROPERTIES); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, + Long.toString(ts + 2)); // Execute at timestamp 2 + Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, + props); + // TODO: this is invalid, as you can't have an array reference in upsert + String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" + + tenantId + "','00A123122312312',2.0d)"; + PreparedStatement statement = conn.prepareStatement(query); + int executeUpdate = statement.executeUpdate(); + assertEquals(1, executeUpdate); + conn.commit(); + statement.close(); + conn.close(); + // create another connection + props = new Properties(TEST_PROPERTIES); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, + Long.toString(ts + 4)); + conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); + query = "SELECT ARRAY_ELEM(a_double_array,3) FROM table_with_array"; + statement = conn.prepareStatement(query); + ResultSet rs = statement.executeQuery(); + assertTrue(rs.next()); + // Need to support primitive + Double[] doubleArr = new Double[1]; + doubleArr[0] = 2.0d; + conn.createArrayOf("DOUBLE", doubleArr); + Double result = rs.getDouble(1); + assertEquals(result, doubleArr[0]); + + } finally { + } } @Test From 090502fbf12fba5342ea41973a0d931545aea027 Mon Sep 17 00:00:00 2001 From: Ramkrishna Date: Tue, 28 Jan 2014 00:05:23 +0530 Subject: [PATCH 3/3] Review comments --- phoenix-core/src/main/antlr3/PhoenixSQL.g | 18 ++- .../org/apache/phoenix/parse/ColumnDef.java | 138 ++++++++++-------- .../org/apache/phoenix/schema/PDataType.java | 26 +--- .../org/apache/phoenix/end2end/ArrayTest.java | 91 ++++-------- 4 files changed, 130 insertions(+), 143 deletions(-) diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g index 5dcfe813..07a89b6a 100644 --- a/phoenix-core/src/main/antlr3/PhoenixSQL.g +++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g @@ -521,11 +521,19 @@ dyn_column_def returns [ColumnDef ret] dyn_column_name_or_def returns [ColumnDef ret] : c=column_name (dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? )? (lsq=LSQUARE (a=NUMBER)? RSQUARE)? - {$ret = factory.columnDef(c, dt, true, - l == null ? null : Integer.parseInt( l.getText() ), - s == null ? null : Integer.parseInt( s.getText() ), - false, - null); } + { + if(lsq != null) + { + throwRecognitionException(lsq); + } else + { + $ret = factory.columnDef(c, dt, true, + l == null ? null : Integer.parseInt( l.getText() ), + s == null ? null : Integer.parseInt( s.getText() ), + false, + null); + } + } ; select_expression returns [SelectStatement ret] diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java index 183da7c7..a2064316 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java @@ -66,64 +66,86 @@ public class ColumnDef { } this.isNull = isNull; - if (this.dataType == PDataType.CHAR) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.VARCHAR) { - if (maxLength != null && maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.DECIMAL) { - Integer origMaxLength = maxLength; - maxLength = maxLength == null ? PDataType.MAX_PRECISION : maxLength; - // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; - if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - // When a precision is specified and a scale is not specified, it is set to 0. - // - // This is the standard as specified in - // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 - // and - // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. - // Otherwise, if scale is bigger than maxLength, just set it to the maxLength; - // - // When neither a precision nor a scale is specified, the precision and scale is - // ignored. All decimal are stored with as much decimal points as possible. - scale = scale == null ? - origMaxLength == null ? null : PDataType.DEFAULT_SCALE : - scale > maxLength ? maxLength : scale; - } else if (this.dataType == PDataType.BINARY) { - if (maxLength == null) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.MISSING_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - if (maxLength < 1) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) - .setColumnName(columnDefName.getColumnName()).build().buildException(); - } - scale = null; - } else if (this.dataType == PDataType.INTEGER) { - maxLength = PDataType.INT_PRECISION; - scale = PDataType.ZERO; - } else if (this.dataType == PDataType.LONG) { - maxLength = PDataType.LONG_PRECISION; - scale = PDataType.ZERO; - } else { - // ignore maxLength and scale for other types. - maxLength = null; - scale = null; - } + if (!this.isArray) { + if (this.dataType == PDataType.CHAR) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.MISSING_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.VARCHAR) { + if (maxLength != null && maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_CHAR_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.DECIMAL) { + Integer origMaxLength = maxLength; + maxLength = maxLength == null ? PDataType.MAX_PRECISION + : maxLength; + // for deciaml, 1 <= maxLength <= PDataType.MAX_PRECISION; + if (maxLength < 1 || maxLength > PDataType.MAX_PRECISION) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.DECIMAL_PRECISION_OUT_OF_RANGE) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + // When a precision is specified and a scale is not + // specified, it is set to 0. + // + // This is the standard as specified in + // http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1832 + // and + // http://docs.oracle.com/javadb/10.6.2.1/ref/rrefsqlj15260.html. + // Otherwise, if scale is bigger than maxLength, just set it + // to the maxLength; + // + // When neither a precision nor a scale is specified, the + // precision and scale is + // ignored. All decimal are stored with as much decimal + // points as possible. + scale = scale == null ? origMaxLength == null ? null + : PDataType.DEFAULT_SCALE + : scale > maxLength ? maxLength : scale; + } else if (this.dataType == PDataType.BINARY) { + if (maxLength == null) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.MISSING_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + if (maxLength < 1) { + throw new SQLExceptionInfo.Builder( + SQLExceptionCode.NONPOSITIVE_BINARY_LENGTH) + .setColumnName(columnDefName.getColumnName()) + .build().buildException(); + } + scale = null; + } else if (this.dataType == PDataType.INTEGER) { + maxLength = PDataType.INT_PRECISION; + scale = PDataType.ZERO; + } else if (this.dataType == PDataType.LONG) { + maxLength = PDataType.LONG_PRECISION; + scale = PDataType.ZERO; + } else { + // ignore maxLength and scale for other types. + maxLength = null; + scale = null; + } + } else { + maxLength = null; + scale = null; + } this.maxLength = maxLength; this.scale = scale; this.isPK = isPK; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java index 8d14fb0c..850ae136 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java @@ -19,32 +19,21 @@ */ package org.apache.phoenix.schema; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.math.MathContext; -import java.math.RoundingMode; -import java.sql.Date; -import java.sql.Time; -import java.sql.Timestamp; -import java.sql.Types; +import java.math.*; +import java.sql.*; import java.text.Format; import java.util.Map; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Base64; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.phoenix.query.KeyRange; +import org.apache.phoenix.query.QueryConstants; +import org.apache.phoenix.util.*; import com.google.common.collect.ImmutableMap; import com.google.common.math.LongMath; -import com.google.common.primitives.Booleans; -import com.google.common.primitives.Doubles; -import com.google.common.primitives.Longs; -import org.apache.phoenix.query.KeyRange; -import org.apache.phoenix.query.QueryConstants; -import org.apache.phoenix.util.ByteUtil; -import org.apache.phoenix.util.DateUtil; -import org.apache.phoenix.util.NumberUtil; -import org.apache.phoenix.util.StringUtil; +import com.google.common.primitives.*; /** @@ -5193,9 +5182,6 @@ public Integer estimateByteSizeFromLength(Integer length) { if (isFixedWidth()) { return getByteSize(); } - if(isArrayType()) { - return null; - } // If not fixed width, default to say the byte size is the same as length. return length; } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java index c9d5f9b4..672f6634 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ArrayTest.java @@ -19,31 +19,18 @@ */ package org.apache.phoenix.end2end; -import static org.apache.phoenix.util.TestUtil.B_VALUE; -import static org.apache.phoenix.util.TestUtil.PHOENIX_JDBC_URL; -import static org.apache.phoenix.util.TestUtil.ROW1; -import static org.apache.phoenix.util.TestUtil.TABLE_WITH_ARRAY; -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.assertNull; -import static org.junit.Assert.assertTrue; +import static org.apache.phoenix.util.TestUtil.*; +import static org.junit.Assert.*; -import java.sql.Array; -import java.sql.Connection; -import java.sql.Date; -import java.sql.DriverManager; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; +import java.sql.*; import java.util.Properties; +import org.apache.phoenix.exception.PhoenixParserException; import org.apache.phoenix.query.BaseTest; import org.apache.phoenix.schema.PhoenixArray; import org.apache.phoenix.util.PhoenixRuntime; import org.apache.phoenix.util.StringUtil; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import com.google.common.primitives.Floats; @@ -438,50 +425,34 @@ public void testUpsertSelectWithColumnRef() throws Exception { } } - @Ignore //TODO: Ram to fix @Test - public void testUpsertSelectWithSelectAsSubQuery3() throws Exception { - long ts = nextTimestamp(); - String tenantId = getOrganizationId(); - createTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - //initTablesWithArrays(tenantId, null, ts, false); - try { - createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), - getDefaultSplits(tenantId), null, ts - 2); - initSimpleArrayTable(tenantId, null, ts, false); - Properties props = new Properties(TEST_PROPERTIES); - props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, - Long.toString(ts + 2)); // Execute at timestamp 2 - Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, - props); - // TODO: this is invalid, as you can't have an array reference in upsert - String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" - + tenantId + "','00A123122312312',2.0d)"; - PreparedStatement statement = conn.prepareStatement(query); - int executeUpdate = statement.executeUpdate(); - assertEquals(1, executeUpdate); - conn.commit(); - statement.close(); - conn.close(); - // create another connection - props = new Properties(TEST_PROPERTIES); - props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, - Long.toString(ts + 4)); - conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); - query = "SELECT ARRAY_ELEM(a_double_array,3) FROM table_with_array"; - statement = conn.prepareStatement(query); - ResultSet rs = statement.executeQuery(); - assertTrue(rs.next()); - // Need to support primitive - Double[] doubleArr = new Double[1]; - doubleArr[0] = 2.0d; - conn.createArrayOf("DOUBLE", doubleArr); - Double result = rs.getDouble(1); - assertEquals(result, doubleArr[0]); - - } finally { - } + // TODO : currently not supported + public void testUpsertSelectWithSpecificIndexOfAnArray() throws Exception { + long ts = nextTimestamp(); + String tenantId = getOrganizationId(); + createTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + // initTablesWithArrays(tenantId, null, ts, false); + Connection conn = null; + try { + createSimpleTableWithArray(BaseConnectedQueryTest.getUrl(), + getDefaultSplits(tenantId), null, ts - 2); + initSimpleArrayTable(tenantId, null, ts, false); + Properties props = new Properties(TEST_PROPERTIES); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, + Long.toString(ts + 2)); // Execute at timestamp 2 + conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props); + // TODO: this is invalid, as you can't have an array reference in + // upsert + String query = "upsert into table_with_array(ORGANIZATION_ID,ENTITY_ID,a_double_array[3]) values('" + + tenantId + "','00A123122312312',2.0d)"; + PreparedStatement statement = conn.prepareStatement(query); + fail("Should have failed with parser Exception"); + } catch (PhoenixParserException e) { + // Correct + } finally { + conn.close(); + } } @Test