DRILL-3243: Better error message when we use an alias in OVER clause
Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/1c9093e0 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/1c9093e0 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/1c9093e0 Branch: refs/heads/master Commit: 1c9093e0f34daaeb1ad6661bb4d4115bc573ed78 Parents: b2bbd99 Author: adeneche <adene...@gmail.com> Authored: Mon Jun 8 16:00:23 2015 -0700 Committer: Hanifi Gunes <hgu...@maprtech.com> Committed: Mon Jul 6 16:28:29 2015 -0700 ---------------------------------------------------------------------- .../drill/common/exceptions/UserException.java | 4 ++++ .../common/exceptions/UserRemoteException.java | 2 +- .../compliant/CompliantTextRecordReader.java | 3 +++ .../text/compliant/RepeatedVarCharOutput.java | 7 +++--- .../exec/store/text/TestNewTextReader.java | 23 +++++++++++++++----- 5 files changed, 30 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/1c9093e0/common/src/main/java/org/apache/drill/common/exceptions/UserException.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java index 13c17bd..b943710 100644 --- a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java +++ b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java @@ -616,6 +616,10 @@ public class UserException extends DrillRuntimeException { return context.getErrorId(); } + public DrillPBError.ErrorType getErrorType() { + return errorType; + } + public String getErrorLocation() { DrillbitEndpoint ep = context.getEndpoint(); if (ep != null) { http://git-wip-us.apache.org/repos/asf/drill/blob/1c9093e0/common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java b/common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java index 1b3fa42..d13fb49 100644 --- a/common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java +++ b/common/src/main/java/org/apache/drill/common/exceptions/UserRemoteException.java @@ -28,7 +28,7 @@ public class UserRemoteException extends UserException { private final DrillPBError error; public UserRemoteException(DrillPBError error) { - super(null, "Drill Remote Exception", null); + super(error.getErrorType(), "Drill Remote Exception", null); this.error = error; } http://git-wip-us.apache.org/repos/asf/drill/blob/1c9093e0/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java index 254e0d8..27b9116 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/CompliantTextRecordReader.java @@ -27,6 +27,7 @@ import javax.annotation.Nullable; import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.common.exceptions.ExecutionSetupException; +import org.apache.drill.common.exceptions.UserException; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.exec.exception.SchemaChangeException; import org.apache.drill.exec.ops.FragmentContext; @@ -112,6 +113,8 @@ public class CompliantTextRecordReader extends AbstractRecordReader { reader.start(); } catch (SchemaChangeException | IOException e) { throw new ExecutionSetupException(String.format("Failure while setting up text reader for file %s", split.getPath()), e); + } catch (IllegalArgumentException e) { + throw UserException.dataReadError(e).addContext("File Path", split.getPath().toString()).build(logger); } } http://git-wip-us.apache.org/repos/asf/drill/blob/1c9093e0/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/RepeatedVarCharOutput.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/RepeatedVarCharOutput.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/RepeatedVarCharOutput.java index 40276f4..91b1a7f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/RepeatedVarCharOutput.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/compliant/RepeatedVarCharOutput.java @@ -132,13 +132,14 @@ class RepeatedVarCharOutput extends TextOutput { if (!isStarQuery) { String pathStr; for (SchemaPath path : columns) { - assert path.getRootSegment().isNamed(); + assert path.getRootSegment().isNamed() : "root segment should be named"; pathStr = path.getRootSegment().getPath(); Preconditions.checkArgument(pathStr.equals(COL_NAME) || (pathStr.equals("*") && path.getRootSegment().getChild() == null), - "Selected column(s) must have name 'columns' or must be plain '*'"); + String.format("Selected column '%s' must have name 'columns' or must be plain '*'", pathStr)); if (path.getRootSegment().getChild() != null) { - Preconditions.checkArgument(path.getRootSegment().getChild().isArray(), "Selected column must be an array index"); + Preconditions.checkArgument(path.getRootSegment().getChild().isArray(), + String.format("Selected column '%s' must be an array index", pathStr)); int index = path.getRootSegment().getChild().getArraySegment().getIndex(); columnIds.add(index); } http://git-wip-us.apache.org/repos/asf/drill/blob/1c9093e0/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestNewTextReader.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestNewTextReader.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestNewTextReader.java index 76674f9..e63e528 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestNewTextReader.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestNewTextReader.java @@ -17,17 +17,19 @@ */ package org.apache.drill.exec.store.text; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.apache.drill.BaseTestQuery; -import org.junit.Assert; +import org.apache.drill.common.exceptions.UserRemoteException; +import org.apache.drill.exec.proto.UserBitShared.DrillPBError.ErrorType; import org.junit.Test; public class TestNewTextReader extends BaseTestQuery { @Test public void fieldDelimiterWithinQuotes() throws Exception { - test("select columns[1] as col1 from cp.`textinput/input1.csv`"); testBuilder() .sqlQuery("select columns[1] as col1 from cp.`textinput/input1.csv`") .unOrdered() @@ -37,14 +39,25 @@ public class TestNewTextReader extends BaseTestQuery { } @Test - public void ensureFailureOnNewLineDelimiterWithinQuotes() throws Exception { + public void ensureFailureOnNewLineDelimiterWithinQuotes() { try { test("select columns[1] as col1 from cp.`textinput/input2.csv`"); + fail("Expected exception not thrown."); } catch (Exception e) { assertTrue(e.getMessage().contains("Cannot use newline character within quoted string")); - return; } - Assert.fail("Expected exception not thrown."); } + @Test + public void ensureColumnNameDisplayedinError() throws Exception { + final String COL_NAME = "col1"; + + try { + test("select max(columns[1]) as %s from cp.`textinput/input1.csv` where %s is not null", COL_NAME, COL_NAME); + fail("Query should have failed"); + } catch(UserRemoteException ex) { + assertEquals(ErrorType.DATA_READ, ex.getErrorType()); + assertTrue("Error message should contain " + COL_NAME, ex.getMessage().contains(COL_NAME)); + } + } }
