Skip to content

Commit

Permalink
TEIID-4446 rationalizing precision/scale metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
@shawkins
shawkins committed Sep 15, 2016
1 parent 12869c0 commit 016d5c7
Show file tree
Hide file tree
Showing 19 changed files with 402 additions and 311 deletions.
28 changes: 25 additions & 3 deletions api/src/main/java/org/teiid/metadata/BaseColumn.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public abstract class BaseColumn extends AbstractMetadataRecord {
public static final String SPATIAL_TYPE = MetadataFactory.SPATIAL_URI + "type"; //$NON-NLS-1$
public static final String SPATIAL_COORD_DIMENSION = MetadataFactory.SPATIAL_URI + "coord_dimension"; //$NON-NLS-1$

//the defaults are safe for odbc/jdbc metadata
public static final int DEFAULT_PRECISION = Short.MAX_VALUE;
//similar to postgresql behavior, we default to a non-zero
public static final int DEFAULT_SCALE = Short.MAX_VALUE/2;

private static final long serialVersionUID = 6382258617714856616L;

public enum NullType {
Expand Down Expand Up @@ -84,12 +89,27 @@ public int getLength() {
}

public int getPrecision() {
if (precision == 0 && this.getDatatype() != null && getDatatype().getName().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
return DEFAULT_PRECISION;
}
return precision;
}

public int getScale() {
if (this.getDatatype() != null && getDatatype().getName().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
if (precision == 0 && scale == 0) {
return DEFAULT_SCALE;
}
if (Math.abs(scale) > precision) {
return Integer.signum(scale)*precision;
}
}
return scale;
}

public boolean isDefaultPrecisionScale() {
return precision == 0 && scale == 0;
}

public int getRadix() {
return radix;
Expand Down Expand Up @@ -174,8 +194,10 @@ public void setDatatype(Datatype datatype, boolean copyAttributes, int arrayDime
if (copyAttributes) {
this.radix = this.datatype.getRadix();
this.length = this.datatype.getLength();
this.precision = this.datatype.getPrecision();
this.scale = this.datatype.getScale();
if (!datatype.getName().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
this.precision = this.datatype.getPrecision();
this.scale = this.datatype.getScale();
}
this.nullType = this.datatype.getNullType();
}
}
Expand Down
2 changes: 1 addition & 1 deletion engine/src/main/java/org/teiid/query/metadata/DDLStringVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ private void appendColumn(BaseColumn column, boolean includeName, boolean includ
append(LPAREN).append(column.getLength()).append(RPAREN);
}
} else if (PRECISION_DATATYPES.contains(runtimeTypeName)
&& (datatype == null || column.getPrecision() != datatype.getPrecision() || column.getScale() != datatype.getScale())) {
&& !column.isDefaultPrecisionScale()) {
append(LPAREN).append(column.getPrecision());
if (column.getScale() != 0) {
append(COMMA).append(column.getScale());
Expand Down
20 changes: 14 additions & 6 deletions engine/src/main/java/org/teiid/query/parser/SQLParserUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,9 @@ private void setColumnOptions(Column c, Map<String, String> props) throws Metada
Map<String, Datatype> datatypes = SystemMetadata.getInstance().getSystemStore().getDatatypes();
if (matcher.matches() && datatypes.get(matcher.group(1)) != null) {
c.setDatatype(datatypes.get(matcher.group(1)));
c.setLength(Integer.parseInt(matcher.group(2)));
c.setPrecision(Integer.parseInt(matcher.group(3)));
c.setScale(Integer.parseInt(matcher.group(4)));
c.setLength(Integer.parseInt(matcher.group(2)));
ParsedDataType pdt = new ParsedDataType(matcher.group(1), Integer.parseInt(matcher.group(3)), Integer.parseInt(matcher.group(4)), true);
setTypeInfo(pdt, c);
}
else {
throw new MetadataException(QueryPlugin.Util.getString("udt_format_wrong", c.getName())); //$NON-NLS-1$
Expand Down Expand Up @@ -848,11 +848,19 @@ void setTypeInfo(ParsedDataType type, BaseColumn column) {
if (type.length != null){
column.setLength(type.length);
}
if (type.scale != null){
column.setScale(type.scale);
}
if (type.precision != null){
if (type.precision == 0) {
throw new MetadataException(QueryPlugin.Util.getString("SQLParser.zero_precision")); //$NON-NLS-1$
}
column.setPrecision(type.precision);
if (type.scale != null){
if (Math.abs(type.scale) > type.precision) {
throw new MetadataException(QueryPlugin.Util.getString("SQLParser.invalid_scale", type.scale, type.precision)); //$NON-NLS-1$
}
column.setScale(type.scale);
} else {
column.setScale(0);
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion engine/src/main/resources/org/teiid/query/i18n.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,9 @@ SQLParser.alter_procedure_param_doesnot_exist=The parameter {0} does not exist o
SQLParser.alter_function_param_doesnot_exist=The parameter {0} does not exist on function {1} in the schema provided.
SQLParser.alter_table_param=Parameter {0} in not a valid alter target on table {1}.
SQLParser.non_position_constant=Order by expression constant {0} is not allowed.
SQLParser.view_doesnot_exist=No view exits with the name {0}.
SQLParser.view_doesnot_exist=No view exits with the name {0}.
SQLParser.zero_precision=Precision cannot be zero.
SQLParser.invalid_scale=Scale {0} cannot be larger than precision {1}.
SystemSource.array_length_desc=Get the length of the given array value
SystemSource.array_param1=Array
SystemSource.array_length_result=The array length
Expand Down
30 changes: 28 additions & 2 deletions engine/src/test/java/org/teiid/query/parser/TestDDLParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,32 @@ public void testForeignTable() throws Exception {
assertEquals("hello", e6.getDefaultValue());
}

@Test(expected=MetadataException.class)
public void testZeroPrecision() throws Exception {
String ddl = "CREATE FOREIGN TABLE G1(e4 decimal(0))";

helpParse(ddl, "model").getSchema();
}

@Test
public void testDefaultPrecision() throws Exception {
String ddl = "CREATE FOREIGN TABLE G1(e4 decimal)";

Schema s = helpParse(ddl, "model").getSchema();
Map<String, Table> tableMap = s.getTables();

assertTrue("Table not found", tableMap.containsKey("G1"));
Table table = tableMap.get("G1");
Column e4 = table.getColumns().get(0);

assertEquals("e4", e4.getName());
assertEquals("bigdecimal", e4.getDatatype().getName());
assertEquals(false, e4.isAutoIncremented());
assertEquals(Short.MAX_VALUE, e4.getPrecision());
assertEquals(Short.MAX_VALUE/2, e4.getScale());
assertEquals(SearchType.Searchable, e4.getSearchType());
}

@Test(expected=MetadataException.class)
public void testDuplicatePrimarykey() throws Exception {
String ddl = "CREATE FOREIGN TABLE G1( e1 integer primary key, e2 varchar primary key)";
Expand All @@ -138,7 +164,7 @@ public void testDuplicatePrimarykey() throws Exception {

@Test
public void testUDT() throws Exception {
String ddl = "CREATE FOREIGN TABLE G1( e1 integer, e2 varchar OPTIONS (UDT 'NMTOKENS(12,13,14)'))";
String ddl = "CREATE FOREIGN TABLE G1( e1 integer, e2 varchar OPTIONS (UDT 'NMTOKENS(12,13,11)'))";

Schema s = helpParse(ddl, "model").getSchema();
Map<String, Table> tableMap = s.getTables();
Expand All @@ -149,7 +175,7 @@ public void testUDT() throws Exception {
assertEquals("NMTOKENS", table.getColumns().get(1).getDatatype().getName());
assertEquals(12, table.getColumns().get(1).getLength());
assertEquals(13, table.getColumns().get(1).getPrecision());
assertEquals(14, table.getColumns().get(1).getScale());
assertEquals(11, table.getColumns().get(1).getScale());
}

@Test public void testFBI() throws Exception {
Expand Down
8 changes: 5 additions & 3 deletions jboss-integration/kits/wildfly/docs/teiid/teiid-releasenotes.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,19 @@ <h2><a name="Compatibility">Compatibility Issues</a></h2>
<li><a href="https://issues.jboss.org/browse/TEIID-3754">TEIID-3754</a> The OData2 web service layer has been deprecated and will be removed in a future release. You should migrate to the OData v4 service layer instead.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4205">TEIID-4205</a> By default, the wrapping begin/commit of a UseDeclareFetch cursor will be ignored as Teiid does not require a transaction. Set the org.teiid.honorDeclareFetchTxn system property to false to revert to the old behavior which honored the transaction.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4240">TEIID-4240</a> The usage of ; delimited statements for materialization scripts has been deprecated. An anonymous procedure block should be used instead if multiple statements are needed.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4228">TEIID-4228</a> Precision and scale values greater than 32767 are deprecated.</li>
</ul>

<h4>from 9.0</h4>
<ul>
<li><a href="https://issues.jboss.org/browse/TEIID-4400">TEIID-4400</a> XML Document Models have been deprecated. OData or SQL/XML should be used instead.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4317">TEIID-4317</a> ExecutionFactory.initCapabilities will always be called - either during start if isSourceRequiredForCapabilities returns false, or later if true.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4346">TEIID-4346</a> The excel-odbc translator has been removed. Please use the excel translator instead.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4332">TEIID-4332</a> Due to costing logic changes plans may be different that in previous releases. Please raise an issue is you feel a plan is not appropriate.
<li><a href="https://issues.jboss.org/browse/TEIID-4421">TEIID-4421</a> Removed the deprecated EmbeddedServer.addTranslator(ExecutionFactory) method.
<li><a href="https://issues.jboss.org/browse/TEIID-4332">TEIID-4332</a> Due to costing logic changes plans may be different that in previous releases. Please raise an issue is you feel a plan is not appropriate.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4421">TEIID-4421</a> Removed the deprecated EmbeddedServer.addTranslator(ExecutionFactory) method.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4442">TEIID-4442</a> Removed the interpretation of the security-domain setting for the session service as a comma separated list of domains. Also added the USER(boolean) function to control
if the USER function returns a name with the security domain. Finally the DatabaseMetaData and CommandContext getUserName will both return the simple user name without the domain.
if the USER function returns a name with the security domain. Finally the DatabaseMetaData and CommandContext getUserName will both return the simple user name without the domain.</li>
<li><a href="https://issues.jboss.org/browse/TEIID-4228">TEIID-4228</a> Precision/scale will now be set consistently. Values reported from JDBC/OData/ODBC metadata may be different if your current metadata declares a bigdecimal type with default precision.</li>
</ul>

<h4>from 8.x</h4>
Expand Down
3 changes: 2 additions & 1 deletion olingo/src/main/java/org/teiid/olingo/service/ODataSQLBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.apache.olingo.commons.api.edm.EdmNavigationProperty;
import org.apache.olingo.commons.api.edm.EdmOperation;
import org.apache.olingo.commons.api.edm.EdmProperty;
import org.apache.olingo.commons.api.ex.ODataRuntimeException;
import org.apache.olingo.commons.core.edm.primitivetype.EdmInt32;
import org.apache.olingo.commons.core.edm.primitivetype.SingletonPrimitiveType;
import org.apache.olingo.server.api.OData;
Expand Down Expand Up @@ -959,7 +960,7 @@ private void visitOperation(EdmOperation operation) {
this.context = cdn;
}
} catch (TeiidProcessingException e) {
this.exceptions.add(e);
throw new ODataRuntimeException(e);
}
}

Expand Down
27 changes: 15 additions & 12 deletions olingo/src/main/java/org/teiid/olingo/service/ODataSchemaBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*/
package org.teiid.olingo.service;

import static org.teiid.language.visitor.SQLStringVisitor.getRecordName;
import static org.teiid.language.visitor.SQLStringVisitor.*;

import java.util.ArrayList;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -185,12 +185,13 @@ private static CsdlProperty buildProperty(Column c, boolean nullable) {

if (c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.STRING)) {
property.setMaxLength(c.getLength()).setUnicode(true);
} else if (c.getRuntimeType().equals(
DataTypeManager.DefaultDataTypes.DOUBLE)
|| c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.FLOAT)
|| c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
property.setPrecision(c.getPrecision());
property.setScale(c.getScale());
} else if (c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
if (c.getScale() < 0) {
property.setPrecision((int)Math.min(Integer.MAX_VALUE, (long)c.getPrecision() - c.getScale()));
} else {
property.setPrecision(c.getPrecision());
}
property.setScale(Math.max(0, c.getScale()));
} else if (c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.TIMESTAMP)
|| c.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.TIME)) {
property.setPrecision(c.getPrecision() == 0?new Integer(4):c.getPrecision());
Expand Down Expand Up @@ -512,11 +513,13 @@ private static CsdlParameter buildParameter(ProcedureParameter pp,

if (pp.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.STRING)) {
param.setMaxLength(pp.getLength());
} else if (pp.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.DOUBLE)
|| pp.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.FLOAT)
|| pp.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
param.setPrecision(pp.getPrecision());
param.setScale(pp.getScale());
} else if (pp.getRuntimeType().equals(DataTypeManager.DefaultDataTypes.BIG_DECIMAL)) {
if (pp.getScale() < 0) {
param.setPrecision((int)Math.min(Integer.MAX_VALUE, (long)pp.getPrecision() - pp.getScale()));
} else {
param.setPrecision(pp.getPrecision());
}
param.setScale(Math.max(0, pp.getScale()));
} else {
if (pp.getDefaultValue() != null) {
//param.setDefaultValue(pp.getDefaultValue());
Expand Down
Loading

0 comments on commit 016d5c7

Please sign in to comment.