Repository: incubator-hawq Updated Branches: refs/heads/HAWQ-992 8b3f416ef -> f7fdd2728
HAWQ-992. Added comments and more tests. Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/f7fdd272 Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/f7fdd272 Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/f7fdd272 Branch: refs/heads/HAWQ-992 Commit: f7fdd2728fea7ded77f2efb2b1151fbaf96ffe48 Parents: 8b3f416 Author: Oleksandr Diachenko <[email protected]> Authored: Mon Aug 29 15:21:09 2016 -0700 Committer: Oleksandr Diachenko <[email protected]> Committed: Mon Aug 29 15:21:09 2016 -0700 ---------------------------------------------------------------------- .../pxf/api/utilities/ColumnDescriptor.java | 62 ++++++++++---------- .../hawq/pxf/api/utilities/EnumHawqType.java | 13 ++-- .../hive/utilities/EnumHiveToHawqType.java | 49 +++++++++++----- .../plugins/hive/utilities/HiveUtilities.java | 7 +-- .../hive/utilities/HiveUtilitiesTest.java | 16 ++--- .../pxf/service/utilities/ProtocolData.java | 25 ++++++-- .../pxf/service/utilities/ProtocolDataTest.java | 59 ++++++++++++++++++- 7 files changed, 157 insertions(+), 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ColumnDescriptor.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ColumnDescriptor.java b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ColumnDescriptor.java index ff85672..5c764c8 100644 --- a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ColumnDescriptor.java +++ b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ColumnDescriptor.java @@ -26,11 +26,11 @@ package org.apache.hawq.pxf.api.utilities; */ public class ColumnDescriptor { - int gpdbColumnTypeCode; - String gpdbColumnName; - String gpdbColumnTypeName; - int gpdbColumnIndex; - String[] gpdbColumnTypeModifiers; + int dbColumnTypeCode; + String dbColumnName; + String dbColumnTypeName; + int dbColumnIndex; + Integer[] dbColumnTypeModifiers; /** * Reserved word for a table record key. @@ -47,12 +47,12 @@ public class ColumnDescriptor { * @param typename type name * @param typemods type modifiers */ - public ColumnDescriptor(String name, int typecode, int index, String typename, String[] typemods) { - gpdbColumnTypeCode = typecode; - gpdbColumnTypeName = typename; - gpdbColumnName = name; - gpdbColumnIndex = index; - gpdbColumnTypeModifiers = typemods; + public ColumnDescriptor(String name, int typecode, int index, String typename, Integer[] typemods) { + dbColumnTypeCode = typecode; + dbColumnTypeName = typename; + dbColumnName = name; + dbColumnIndex = index; + dbColumnTypeModifiers = typemods; } /** @@ -61,50 +61,50 @@ public class ColumnDescriptor { * @param copy the ColumnDescriptor to copy */ public ColumnDescriptor(ColumnDescriptor copy) { - this.gpdbColumnTypeCode = copy.gpdbColumnTypeCode; - this.gpdbColumnName = copy.gpdbColumnName; - this.gpdbColumnIndex = copy.gpdbColumnIndex; - this.gpdbColumnTypeName = copy.gpdbColumnTypeName; - System.arraycopy(this.gpdbColumnTypeModifiers, 0, - copy.gpdbColumnTypeModifiers, 0, - this.gpdbColumnTypeModifiers.length); + this.dbColumnTypeCode = copy.dbColumnTypeCode; + this.dbColumnName = copy.dbColumnName; + this.dbColumnIndex = copy.dbColumnIndex; + this.dbColumnTypeName = copy.dbColumnTypeName; + System.arraycopy(this.dbColumnTypeModifiers, 0, + copy.dbColumnTypeModifiers, 0, + this.dbColumnTypeModifiers.length); } public String columnName() { - return gpdbColumnName; + return dbColumnName; } public int columnTypeCode() { - return gpdbColumnTypeCode; + return dbColumnTypeCode; } public int columnIndex() { - return gpdbColumnIndex; + return dbColumnIndex; } public String columnTypeName() { - return gpdbColumnTypeName; + return dbColumnTypeName; } - public String[] columnTypeModifiers() { - return gpdbColumnTypeModifiers; + public Integer[] columnTypeModifiers() { + return dbColumnTypeModifiers; } /** - * Returns <tt>true</tt> if {@link #gpdbColumnName} is a {@link #RECORD_KEY_NAME}. + * Returns <tt>true</tt> if {@link #dbColumnName} is a {@link #RECORD_KEY_NAME}. * * @return whether column is a record key column */ public boolean isKeyColumn() { - return RECORD_KEY_NAME.equalsIgnoreCase(gpdbColumnName); + return RECORD_KEY_NAME.equalsIgnoreCase(dbColumnName); } @Override public String toString() { - return "ColumnDescriptor [gpdbColumnTypeCode=" + gpdbColumnTypeCode - + ", gpdbColumnName=" + gpdbColumnName - + ", gpdbColumnTypeName=" + gpdbColumnTypeName - + ", gpdbColumnIndex=" + gpdbColumnIndex - + ", gpdbColumnTypeModifiers=" + gpdbColumnTypeModifiers + "]"; + return "ColumnDescriptor [dbColumnTypeCode=" + dbColumnTypeCode + + ", dbColumnName=" + dbColumnName + + ", dbColumnTypeName=" + dbColumnTypeName + + ", dbColumnIndex=" + dbColumnIndex + + ", dbColumnTypeModifiers=" + dbColumnTypeModifiers + "]"; } } http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/EnumHawqType.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/EnumHawqType.java b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/EnumHawqType.java index f35fa5e..1187069 100644 --- a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/EnumHawqType.java +++ b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/EnumHawqType.java @@ -76,7 +76,7 @@ public enum EnumHawqType { EnumHawqType(String typeName, DataType dataType, byte modifiersNum, boolean mandatoryModifiers) { this(typeName, dataType, modifiersNum); - this.setMandatoryModifiers(mandatoryModifiers); + this.mandatoryModifiers = mandatoryModifiers; } /** @@ -98,19 +98,18 @@ public enum EnumHawqType { /** * * @return data type + * @see DataType */ public DataType getDataType() { return this.dataType; } + /** + * + * @return whether modifiers are mandatory for this type + */ public boolean isMandatoryModifiers() { return mandatoryModifiers; } - public void setMandatoryModifiers(boolean mandatoryModifiers) { - this.mandatoryModifiers = mandatoryModifiers; - } } - - - http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/EnumHiveToHawqType.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/EnumHiveToHawqType.java b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/EnumHiveToHawqType.java index 9b24642..f90f39a 100644 --- a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/EnumHiveToHawqType.java +++ b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/EnumHiveToHawqType.java @@ -66,7 +66,7 @@ public enum EnumHiveToHawqType { EnumHiveToHawqType(String typeName, EnumHawqType hawqType, byte size) { this(typeName, hawqType); - this.setSize(size); + this.size = size; } EnumHiveToHawqType(String typeName, EnumHawqType hawqType, String splitExpression) { @@ -122,14 +122,22 @@ public enum EnumHiveToHawqType { + hiveType + " to HAWQ's type"); } - public static EnumHiveToHawqType getCompatibleHawqToHiveType(DataType dataType) { - SortedSet<EnumHiveToHawqType> types = new TreeSet<EnumHiveToHawqType>(new Comparator<EnumHiveToHawqType>() { + /** + * + * @param dataType Hawq data type + * @return compatible Hive type to given Hawq type, if there are more than one compatible types, it returns one with bigger size + * @throws UnsupportedTypeException if there is no corresponding Hive type for given Hawq type + */ + public static EnumHiveToHawqType getCompatibleHawqToHiveType(DataType dataType) { - public int compare(EnumHiveToHawqType a, EnumHiveToHawqType b){ - return Byte.compare(a.getSize(), b.getSize()); - } - }); + SortedSet<EnumHiveToHawqType> types = new TreeSet<EnumHiveToHawqType>( + new Comparator<EnumHiveToHawqType>() { + public int compare(EnumHiveToHawqType a, + EnumHiveToHawqType b) { + return Byte.compare(a.getSize(), b.getSize()); + } + }); for (EnumHiveToHawqType t : values()) { if (t.getHawqType().getDataType().equals(dataType)) { @@ -138,23 +146,31 @@ public enum EnumHiveToHawqType { } if (types.size() == 0) - throw new UnsupportedTypeException("Unable to map HAWQ's type: " - + dataType + " to Hive's type"); + throw new UnsupportedTypeException("Unable to find compatible Hive type for given HAWQ's type: " + dataType); return types.last(); } - public static String[] extractModifiers(String hiveType) { - String[] result = null; + /** + * + * @param hiveType full Hive data type, i.e. varchar(10) etc + * @return array of type modifiers + * @throws UnsupportedTypeException if there is no such Hive type supported + */ + public static Integer[] extractModifiers(String hiveType) { + Integer[] result = null; for (EnumHiveToHawqType t : values()) { String hiveTypeName = hiveType; String splitExpression = t.getSplitExpression(); if (splitExpression != null) { String[] tokens = hiveType.split(splitExpression); hiveTypeName = tokens[0]; - result = Arrays.copyOfRange(tokens, 1, tokens.length); + result = new Integer[tokens.length - 1]; + for (int i = 0; i < tokens.length - 1; i++) + result[i] = Integer.parseInt(tokens[i+1]); } - if (t.getTypeName().toLowerCase().equals(hiveTypeName.toLowerCase())) { + if (t.getTypeName().toLowerCase() + .equals(hiveTypeName.toLowerCase())) { return result; } } @@ -162,11 +178,12 @@ public enum EnumHiveToHawqType { + hiveType + " to HAWQ's type"); } + /** + * This field is needed to find compatible Hive type when more than one Hive type mapped to HAWQ type + * @return size of this type in bytes or 0 + */ public byte getSize() { return size; } - public void setSize(byte size) { - this.size = size; - } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilities.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilities.java b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilities.java index 6bda9b7..7a74ca6 100644 --- a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilities.java +++ b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilities.java @@ -287,7 +287,7 @@ public class HiveUtilities { - public static void validateTypeCompatible(DataType hawqDataType, String[] hawqTypeMods, String hiveType, String hawqColumnName) { + public static void validateTypeCompatible(DataType hawqDataType, Integer[] hawqTypeMods, String hiveType, String hawqColumnName) { EnumHiveToHawqType hiveToHawqType = EnumHiveToHawqType.getHiveToHawqType(hiveType); EnumHawqType expectedHawqType = hiveToHawqType.getHawqType(); @@ -297,10 +297,9 @@ public class HiveUtilities { switch (hawqDataType) { case NUMERIC: - String[] hiveTypeModifiers = EnumHiveToHawqType.extractModifiers(hiveType); + Integer[] hiveTypeModifiers = EnumHiveToHawqType.extractModifiers(hiveType); for (int i = 0; hawqTypeMods != null && i < hawqTypeMods.length; i++) { - if (Integer.valueOf(hawqTypeMods[i]) < Integer - .valueOf(hiveTypeModifiers[i])) + if (hawqTypeMods[i] < hiveTypeModifiers[i]) throw new UnsupportedTypeException( "Invalid definition for column " + hawqColumnName + ": modifiers are not compatible, " http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilitiesTest.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilitiesTest.java b/pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilitiesTest.java index ea1cac1..799c8be 100644 --- a/pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilitiesTest.java +++ b/pxf/pxf-hive/src/test/java/org/apache/hawq/pxf/plugins/hive/utilities/HiveUtilitiesTest.java @@ -135,18 +135,18 @@ public class HiveUtilitiesTest { public void validateSchema() throws Exception { String columnName = "abc"; - String[] hawqModifiers = {}; + Integer[] hawqModifiers = {}; HiveUtilities.validateTypeCompatible(DataType.SMALLINT, hawqModifiers, EnumHiveToHawqType.TinyintType.getTypeName(), columnName); HiveUtilities.validateTypeCompatible(DataType.SMALLINT, hawqModifiers, EnumHiveToHawqType.SmallintType.getTypeName(), columnName); //Both Hive and HAWQ types have the same modifiers - hawqModifiers = new String[]{"38", "18"}; + hawqModifiers = new Integer[]{38, 18}; HiveUtilities.validateTypeCompatible(DataType.NUMERIC, hawqModifiers, "decimal(38,18)", columnName); //HAWQ datatype doesn't require modifiers, they are empty, Hive has non-empty modifiers //Types are compatible in this case - hawqModifiers = new String[]{}; + hawqModifiers = new Integer[]{}; HiveUtilities.validateTypeCompatible(DataType.NUMERIC, hawqModifiers, "decimal(38,18)", columnName); hawqModifiers = null; HiveUtilities.validateTypeCompatible(DataType.NUMERIC, hawqModifiers, "decimal(38,18)", columnName); @@ -154,7 +154,7 @@ public class HiveUtilitiesTest { //HAWQ datatype requires modifiers but they aren't provided //Types aren't compatible try { - hawqModifiers = new String[]{}; + hawqModifiers = new Integer[]{}; HiveUtilities.validateTypeCompatible(DataType.VARCHAR, hawqModifiers, "varchar", columnName); fail("should fail with incompatible modifiers message"); } @@ -164,13 +164,13 @@ public class HiveUtilitiesTest { } //HAWQ has wider modifiers than Hive, types are compatible - hawqModifiers = new String[]{"11", "3"}; + hawqModifiers = new Integer[]{11, 3}; HiveUtilities.validateTypeCompatible(DataType.NUMERIC, hawqModifiers, "decimal(10,2)", columnName); //HAWQ has lesser modifiers than Hive, types aren't compatible try { - hawqModifiers = new String[]{"38", "17"}; + hawqModifiers = new Integer[]{38, 17}; HiveUtilities.validateTypeCompatible(DataType.NUMERIC, hawqModifiers, "decimal(38,18)", columnName); fail("should fail with incompatible modifiers message"); } @@ -185,8 +185,8 @@ public class HiveUtilitiesTest { @Test public void extractModifiers() throws Exception { - String[] mods = EnumHiveToHawqType.extractModifiers("decimal(10,2)"); - assertEquals(mods, new String[]{"10", "2"}); + Integer[] mods = EnumHiveToHawqType.extractModifiers("decimal(10,2)"); + assertEquals(mods, new Integer[]{10, 2}); } @Test http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java index 88d3139..1797b88 100644 --- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java +++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java @@ -389,7 +389,7 @@ public class ProtocolData extends InputData { String columnName = getProperty("ATTR-NAME" + i); int columnTypeCode = getIntProperty("ATTR-TYPECODE" + i); String columnTypeName = getProperty("ATTR-TYPENAME" + i); - String[] columnTypeMods = parseTypeMods(i); + Integer[] columnTypeMods = parseTypeMods(i); ColumnDescriptor column = new ColumnDescriptor(columnName, columnTypeCode, i, columnTypeName, columnTypeMods); @@ -401,14 +401,27 @@ public class ProtocolData extends InputData { } } - private String[] parseTypeMods(int columnIndex) { + private Integer[] parseTypeMods(int columnIndex) { String typeModeCountStr = getOptionalProperty("ATTR-TYPEMOD" + columnIndex + "-COUNT"); - String[] result = null; + Integer[] result = null; + Integer typeModeCount = null; if (typeModeCountStr != null) { - Integer typeModeCount = Integer.parseInt(typeModeCountStr); - result = new String[typeModeCount]; + try { + typeModeCount = Integer.parseInt(typeModeCountStr); + if (typeModeCount < 0) + throw new IllegalArgumentException("ATTR-TYPEMOD" + columnIndex + "-COUNT cann't be negative"); + result = new Integer[typeModeCount]; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("ATTR-TYPEMOD" + columnIndex + "-COUNT must be a positive integer"); + } for (int i = 0; i < typeModeCount; i++) { - result[i] = getProperty("ATTR-TYPEMOD" + columnIndex + "-" + i); + try { + result[i] = Integer.parseInt(getProperty("ATTR-TYPEMOD" + columnIndex + "-" + i)); + if (result[i] < 0) + throw new NumberFormatException(); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("ATTR-TYPEMOD" + columnIndex + "-" + i + " must be a positive integer"); + } } } return result; http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/f7fdd272/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java ---------------------------------------------------------------------- diff --git a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java index 03860f9..09efe81 100644 --- a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java +++ b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java @@ -351,8 +351,63 @@ public class ProtocolDataTest { ProtocolData protocolData = new ProtocolData(parameters); - assertEquals(protocolData.getColumn(0).columnTypeModifiers(), new String[]{"5"}); - assertEquals(protocolData.getColumn(1).columnTypeModifiers(), new String[]{"10", "2"}); + assertEquals(protocolData.getColumn(0).columnTypeModifiers(), new Integer[]{5}); + assertEquals(protocolData.getColumn(1).columnTypeModifiers(), new Integer[]{10, 2}); + } + + @Test + public void typeModsNegative() { + + parameters.put("X-GP-ATTRS", "1"); + parameters.put("X-GP-ATTR-NAME0", "vc1"); + parameters.put("X-GP-ATTR-TYPECODE0", "1043"); + parameters.put("X-GP-ATTR-TYPENAME0", "varchar"); + parameters.put("X-GP-ATTR-TYPEMOD0-COUNT", "X"); + parameters.put("X-GP-ATTR-TYPEMOD0-0", "Y"); + + + try { + ProtocolData protocolData = new ProtocolData(parameters); + fail("should throw IllegalArgumentException when bad value received for X-GP-ATTR-TYPEMOD0-COUNT"); + } catch (IllegalArgumentException iae) { + assertEquals( + "ATTR-TYPEMOD0-COUNT must be a positive integer", + iae.getMessage()); + } + + parameters.put("X-GP-ATTR-TYPEMOD0-COUNT", "-1"); + + try { + ProtocolData protocolData = new ProtocolData(parameters); + fail("should throw IllegalArgumentException when negative value received for X-GP-ATTR-TYPEMOD0-COUNT"); + } catch (IllegalArgumentException iae) { + assertEquals( + "ATTR-TYPEMOD0-COUNT cann't be negative", + iae.getMessage()); + } + + parameters.put("X-GP-ATTR-TYPEMOD0-COUNT", "1"); + + try { + ProtocolData protocolData = new ProtocolData(parameters); + fail("should throw IllegalArgumentException when bad value received for X-GP-ATTR-TYPEMOD0-0"); + } catch (IllegalArgumentException iae) { + assertEquals( + "ATTR-TYPEMOD0-0 must be a positive integer", + iae.getMessage()); + } + + parameters.put("X-GP-ATTR-TYPEMOD0-COUNT", "2"); + parameters.put("X-GP-ATTR-TYPEMOD0-0", "42"); + + try { + ProtocolData protocolData = new ProtocolData(parameters); + fail("should throw IllegalArgumentException number of actual type modifiers is less than X-GP-ATTR-TYPEMODX-COUNT"); + } catch (IllegalArgumentException iae) { + assertEquals( + "Internal server error. Property \"ATTR-TYPEMOD0-1\" has no value in current request", + iae.getMessage()); + } } /*
