Master apache #719
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,64 +66,86 @@ public class ColumnDef { | |
| } | ||
|
|
||
| this.isNull = isNull; | ||
| if (!this.isArray) { | ||
|
There was a problem hiding this comment. This would be bad. We need the maxLength for arrays too. It represents the maxLength of the element (i.e. the precision for a DECIMAL or the max length for a CHAR). There was a problem hiding this comment. As i am returning getByteSize as null, this needs to be changed or add There was a problem hiding this comment. I'd recommend working on #669. I need to do a bit of clean up (there's There was a problem hiding this comment. Got the code from the apache git repo now. this has been left as it is. If you are ok with this then am fine. Will start on #669 There was a problem hiding this comment. That's probably a good fix to make. You have the length passed into that On Mon, Jan 27, 2014 at 11:21 PM, ramkrish86 [email protected]:
|
||
| 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; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is always an error, then we wouldn't want it in the syntax, but I'm actually capturing this information now and storing it in an ARRAY_SIZE key value column on the column metadata row. We're not using it, but we should use it down the road.
With our dynamic column feature, you can define a column at upsert time and populate it. This would include arrays. I think for arrays, you should never have a null dt. I think it's just a matter of not having both the dt part and the array brackets be optional - instead the whole thing should be optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for now I will revert this change? I was just wondering how that
bq.(lsq=LSQUARE (a=NUMBER)? RSQUARE)?
was added. I was thinking I added that in my previous commit. So you don't want that to be solved in the syntax level? Ok will see then what to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure you switch to the Apache git repo as the github repo is no longer
being used - your on the phoenix dev list, right and have been getting my
mail? If not please join here:
http://phoenix.incubator.apache.org/mailing_list.html
Get the latest from the Apache git repo and you'll see my fix - it was just
modifying the syntax slightly.
For PDataType.getByteSize() should return null if the type doesn't know how
big it is, which is fine for arrays. For the Expression (i.e. this would be
a KeyValueColumnExpression specifically), the getByteSize() has all the
information it needs to calculate the size correctly.