drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meh...@apache.org
Subject [1/3] drill git commit: DRILL-1488: Provide configurable option to set the sql identifier max length which is passed to Calcite's SQL parser. Main fix is in Calcite version 0.9-drill-r9 (see CALCITE-464).
Date Mon, 01 Dec 2014 03:05:19 GMT
Repository: drill
Updated Branches:
  refs/heads/merge_14_11_30_1 [created] 3d836b518


DRILL-1488: Provide configurable option to set the sql identifier max length which is passed
to Calcite's SQL parser. Main fix is in Calcite version 0.9-drill-r9 (see CALCITE-464).


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/6483eb47
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/6483eb47
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/6483eb47

Branch: refs/heads/merge_14_11_30_1
Commit: 6483eb47769cdc192f11682b35da36cd5e23c81d
Parents: 3a067cf
Author: Aman Sinha <asinha@mapr.com>
Authored: Wed Nov 19 20:41:10 2014 -0800
Committer: Aman Sinha <asinha@maprtech.com>
Committed: Sat Nov 29 15:55:52 2014 -0800

----------------------------------------------------------------------
 exec/java-exec/src/main/codegen/data/Parser.tdd |  7 ++++---
 .../exec/planner/physical/PlannerSettings.java  |  9 +++++++++
 .../drill/exec/planner/sql/DrillSqlWorker.java  |  9 +++++++--
 .../DrillParserWithCompoundIdConverter.java     |  6 +++++-
 .../server/options/SystemOptionManager.java     |  1 +
 .../exec/server/options/TypeValidators.java     | 21 ++++++++++++++++++++
 .../org/apache/drill/TestExampleQueries.java    | 10 ++++++++++
 .../exec/sql/TestSqlBracketlessSyntax.java      |  4 +++-
 pom.xml                                         |  2 +-
 9 files changed, 61 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/codegen/data/Parser.tdd
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/data/Parser.tdd b/exec/java-exec/src/main/codegen/data/Parser.tdd
index de7af2a..5485079 100644
--- a/exec/java-exec/src/main/codegen/data/Parser.tdd
+++ b/exec/java-exec/src/main/codegen/data/Parser.tdd
@@ -66,7 +66,8 @@
     "parserImpls.ftl"
   ]
   
-  includeCompoundIdentifier: false
-  
-  
+  includeCompoundIdentifier: false,
+
+  identifierMaxLength: 1024
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
index 38274c7..faa8546 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
@@ -25,6 +25,7 @@ import org.apache.drill.exec.server.options.TypeValidators.BooleanValidator;
 import org.apache.drill.exec.server.options.TypeValidators.LongValidator;
 import org.apache.drill.exec.server.options.TypeValidators.PositiveLongValidator;
 import org.apache.drill.exec.server.options.TypeValidators.RangeDoubleValidator;
+import org.apache.drill.exec.server.options.TypeValidators.RangeLongValidator;
 import org.eigenbase.relopt.Context;
 
 public class PlannerSettings implements Context{
@@ -34,6 +35,7 @@ public class PlannerSettings implements Context{
   private boolean useDefaultCosting = false; // True: use default Optiq costing, False: use
Drill costing
 
   public static final int MAX_BROADCAST_THRESHOLD = Integer.MAX_VALUE;
+  public static final int DEFAULT_IDENTIFIER_MAX_LENGTH = 1024;
 
   public static final OptionValidator EXCHANGE = new BooleanValidator("planner.disable_exchanges",
false);
   public static final OptionValidator HASHAGG = new BooleanValidator("planner.enable_hashagg",
true);
@@ -47,6 +49,9 @@ public class PlannerSettings implements Context{
   public static final OptionValidator PRODUCER_CONSUMER = new BooleanValidator("planner.add_producer_consumer",
false);
   public static final OptionValidator PRODUCER_CONSUMER_QUEUE_SIZE = new LongValidator("planner.producer_consumer_queue_size",
10);
   public static final OptionValidator HASH_SINGLE_KEY = new BooleanValidator("planner.enable_hash_single_key",
true);
+  public static final OptionValidator IDENTIFIER_MAX_LENGTH =
+      new RangeLongValidator("planner.identifier_max_length", 128 /* A minimum length is
needed because option names are identifiers themselves */,
+                              Integer.MAX_VALUE, DEFAULT_IDENTIFIER_MAX_LENGTH);
 
   public OptionManager options = null;
   public FunctionImplementationRegistry functionImplementationRegistry = null;
@@ -128,6 +133,10 @@ public class PlannerSettings implements Context{
     return options.getOption(ExecConstants.FILESYSTEM_PARTITION_COLUMN_LABEL).string_val;
   }
 
+  public long getIdentifierMaxLength(){
+    return options.getOption(IDENTIFIER_MAX_LENGTH.getOptionName()).num_val;
+  }
+
   @Override
   public <T> T unwrap(Class<T> clazz) {
     if(clazz == PlannerSettings.class){

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
index 863a6dc..5506e92 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
@@ -18,6 +18,7 @@
 package org.apache.drill.exec.planner.sql;
 
 import java.io.IOException;
+import java.io.Reader;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -53,7 +54,10 @@ import org.eigenbase.relopt.RelTraitDef;
 import org.eigenbase.relopt.hep.HepPlanner;
 import org.eigenbase.relopt.hep.HepProgramBuilder;
 import org.eigenbase.sql.SqlNode;
+import org.eigenbase.sql.parser.SqlAbstractParserImpl;
 import org.eigenbase.sql.parser.SqlParseException;
+import org.eigenbase.sql.parser.SqlParser;
+import org.eigenbase.sql.parser.SqlParserImplFactory;
 
 public class DrillSqlWorker {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillSqlWorker.class);
@@ -64,7 +68,6 @@ public class DrillSqlWorker {
   public final static int PHYSICAL_MEM_RULES = 1;
   private final QueryContext context;
 
-
   public DrillSqlWorker(QueryContext context) {
     final List<RelTraitDef> traitDefs = new ArrayList<RelTraitDef>();
 
@@ -74,8 +77,10 @@ public class DrillSqlWorker {
     this.context = context;
     RelOptCostFactory costFactory = (context.getPlannerSettings().useDefaultCosting()) ?
         null : new DrillCostBase.DrillCostFactory() ;
+    int idMaxLength = (int)context.getPlannerSettings().getIdentifierMaxLength();
+
     FrameworkConfig config = Frameworks.newConfigBuilder() //
-        .lex(Lex.MYSQL) //
+        .parserConfig(new SqlParser.ParserConfigImpl(Lex.MYSQL, idMaxLength)) //
         .parserFactory(DrillParserWithCompoundIdConverter.FACTORY) //
         .defaultSchema(context.getNewDefaultSchema()) //
         .operatorTable(context.getDrillOperatorTable()) //

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/impl/DrillParserWithCompoundIdConverter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/impl/DrillParserWithCompoundIdConverter.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/impl/DrillParserWithCompoundIdConverter.java
index 18f1edc..be333c0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/impl/DrillParserWithCompoundIdConverter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/impl/DrillParserWithCompoundIdConverter.java
@@ -19,6 +19,7 @@ package org.apache.drill.exec.planner.sql.parser.impl;
 
 import java.io.Reader;
 
+import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.planner.sql.parser.CompoundIdentifierConverter;
 import org.eigenbase.sql.SqlNode;
 import org.eigenbase.sql.parser.SqlAbstractParserImpl;
@@ -30,9 +31,12 @@ public class DrillParserWithCompoundIdConverter extends DrillParserImpl
{
   /**
    * {@link org.eigenbase.sql.parser.SqlParserImplFactory} implementation for creating parser.
    */
+
   public static final SqlParserImplFactory FACTORY = new SqlParserImplFactory() {
     public SqlAbstractParserImpl getParser(Reader stream) {
-      return new DrillParserWithCompoundIdConverter(stream);
+      SqlAbstractParserImpl parserImpl = new DrillParserWithCompoundIdConverter(stream);
+      parserImpl.setIdentifierMaxLength(PlannerSettings.DEFAULT_IDENTIFIER_MAX_LENGTH);
+      return parserImpl;
     }
   };
 

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
index 1622d7b..34729e2 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
@@ -53,6 +53,7 @@ public class SystemOptionManager implements OptionManager {
       PlannerSettings.PRODUCER_CONSUMER,
       PlannerSettings.PRODUCER_CONSUMER_QUEUE_SIZE,
       PlannerSettings.HASH_SINGLE_KEY,
+      PlannerSettings.IDENTIFIER_MAX_LENGTH,
       ExecConstants.OUTPUT_FORMAT_VALIDATOR,
       ExecConstants.PARQUET_BLOCK_SIZE_VALIDATOR,
       ExecConstants.PARQUET_VECTOR_FILL_THRESHOLD_VALIDATOR,

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
index a0afd29..14be4b0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java
@@ -112,6 +112,27 @@ public class TypeValidators {
     }
   }
 
+  public static class RangeLongValidator extends LongValidator {
+    private final long min;
+    private final long max;
+
+    public RangeLongValidator(String name, long min, long max, long def) {
+      super(name, def);
+      this.min = min;
+      this.max = max;
+    }
+
+    @Override
+    public void validate(OptionValue v) throws ExpressionParsingException {
+      super.validate(v);
+      if (v.num_val > max || v.num_val < min) {
+        throw new ExpressionParsingException(String.format("Option %s must be between %d
and %d.", getOptionName(), min,
+            max));
+      }
+    }
+
+  }
+
   public static abstract class TypeValidator extends OptionValidator {
     final Kind kind;
     private OptionValue defaultValue;

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
index 29495ad..3caede7 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
@@ -572,4 +572,14 @@ public class TestExampleQueries extends BaseTestQuery{
         expectedRecordCount, actualRecordCount), expectedRecordCount, actualRecordCount);
   }
 
+  @Test // DRILL-1488
+  public void testIdentifierMaxLength() throws  Exception {
+    // use long column alias name (approx 160 chars)
+    test("select employee_id as  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
from cp.`employee.json` limit 1");
+
+    // use long table alias name  (approx 160 chars)
+    test("select employee_id from cp.`employee.json` as aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
limit 1");
+
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSqlBracketlessSyntax.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSqlBracketlessSyntax.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSqlBracketlessSyntax.java
index f0ef709..a5bcdec 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSqlBracketlessSyntax.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestSqlBracketlessSyntax.java
@@ -23,11 +23,13 @@ import net.hydromatic.optiq.tools.FrameworkConfig;
 import net.hydromatic.optiq.tools.Frameworks;
 import net.hydromatic.optiq.tools.Planner;
 
+import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.exec.planner.sql.DrillConvertletTable;
 import org.apache.drill.exec.planner.sql.parser.CompoundIdentifierConverter;
 import org.apache.drill.exec.planner.sql.parser.impl.DrillParserImpl;
 import org.apache.drill.test.DrillAssert;
 import org.eigenbase.sql.SqlNode;
+import org.eigenbase.sql.parser.SqlParser;
 import org.junit.Test;
 
 public class TestSqlBracketlessSyntax {
@@ -36,7 +38,7 @@ public class TestSqlBracketlessSyntax {
   @Test
   public void checkComplexExpressionParsing() throws Exception{
     FrameworkConfig config = Frameworks.newConfigBuilder() //
-        .lex(Lex.MYSQL) //
+        .parserConfig(new SqlParser.ParserConfigImpl(Lex.MYSQL, PlannerSettings.DEFAULT_IDENTIFIER_MAX_LENGTH))
         .parserFactory(DrillParserImpl.FACTORY) //
         .defaultSchema(SimpleOptiqSchema.createRootSchema(false)) //
         .convertletTable(new DrillConvertletTable()) //

http://git-wip-us.apache.org/repos/asf/drill/blob/6483eb47/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 4308c2a..d22984d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -920,7 +920,7 @@
           <dependency>
             <groupId>net.hydromatic</groupId>
             <artifactId>optiq-core</artifactId>
-            <version>0.9-drill-r8</version>
+            <version>0.9-drill-r9</version>
             <exclusions>
               <exclusion>
                 <groupId>org.jgrapht</groupId>


Mime
View raw message