You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/07/11 05:53:59 UTC
[1/4] impala git commit: Fix zsh issue in set-pythonpath.sh
Repository: impala
Updated Branches:
refs/heads/master c01efd096 -> 36ce54bd5
Fix zsh issue in set-pythonpath.sh
When sourcing set-pythonpath.sh on a partially checked out or partially
built source tree (e.g. when running perf tests or after build errors),
an empty shell glob pattern would return an empty list in bash but
trigger an error in zsh. This would lead to the script aborting before
exporting the PYTHONPATH variable, leading to hard to debug test
failures.
To fix this, this change replaces the glob pattern with a call to
'find', which works independent of the shell being used.
Although we would only hit this in somewhat contrived edge cases, it
seems beneficial to make the code work as expected on all shells.
Testing:
I built Impala, sourced impala-config.sh in both new zsh and bash
shells, and observed that PYTHONPATH was exported correctly.
Then I deleted the .egg files in question, sourced impala-config.sh in
both new zsh and bash shells, and observed that PYTHONPATH was exported
correctly, too.
Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Reviewed-on: http://gerrit.cloudera.org:8080/10901
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/9abd1fea
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/9abd1fea
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/9abd1fea
Branch: refs/heads/master
Commit: 9abd1fea0ef520cc5d15512c972148193e0fbb1d
Parents: c01efd0
Author: Lars Volker <lv...@cloudera.com>
Authored: Tue Jul 10 09:27:12 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 01:43:42 2018 +0000
----------------------------------------------------------------------
bin/set-pythonpath.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/9abd1fea/bin/set-pythonpath.sh
----------------------------------------------------------------------
diff --git a/bin/set-pythonpath.sh b/bin/set-pythonpath.sh
index 8dcec1d..10d0c67 100755
--- a/bin/set-pythonpath.sh
+++ b/bin/set-pythonpath.sh
@@ -36,7 +36,7 @@ done
PYTHONPATH=${PYTHONPATH}:${HIVE_HOME}/lib/py
# Add all the built eggs to the python path
-for EGG in ${IMPALA_HOME}/shell/ext-py/*/dist/*.egg; do
+for EGG in `find ${IMPALA_HOME}/shell/ext-py/ -name '*\.egg'`; do
PYTHONPATH=${PYTHONPATH}:${EGG}
done
[3/4] impala git commit: IMPALA-7216: Fix SQL generated by
CreateViewStmt & AlterViewStmt
Posted by ta...@apache.org.
IMPALA-7216: Fix SQL generated by CreateViewStmt & AlterViewStmt
The toSql functions in CreateViewStmt and AlterViewStmt generated
invalid SQL by appending types to column definitions. This change
appends just the column names to fix it.
Testing: Added tests to ToSqlTest to verify it.
Change-Id: Ie2a834d4d8d11ba18262d2828f4829c80ed05190
Reviewed-on: http://gerrit.cloudera.org:8080/10886
Reviewed-by: Bikramjeet Vig <bi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/c465150b
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/c465150b
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/c465150b
Branch: refs/heads/master
Commit: c465150ba94a5d43e949344b4f4444124a8738a3
Parents: f164366
Author: poojanilangekar <po...@cloudera.com>
Authored: Thu Jul 5 17:47:44 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 02:41:16 2018 +0000
----------------------------------------------------------------------
.../apache/impala/analysis/AlterViewStmt.java | 3 +-
.../analysis/CreateOrAlterViewStmtBase.java | 14 +++++
.../apache/impala/analysis/CreateViewStmt.java | 3 +-
.../org/apache/impala/analysis/ToSqlTest.java | 65 ++++++++++++++++++++
4 files changed, 81 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
index cad0169..910dd98 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
@@ -28,7 +28,6 @@ import org.apache.impala.thrift.TAccessEvent;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.service.BackendConfig;
-import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
/**
@@ -73,7 +72,7 @@ public class AlterViewStmt extends CreateOrAlterViewStmtBase {
sb.append(tableName_.getDb() + ".");
}
sb.append(tableName_.getTbl());
- if (columnDefs_ != null) sb.append("(" + Joiner.on(", ").join(columnDefs_) + ")");
+ if (columnDefs_ != null) sb.append("(" + getColumnNames() + ")");
sb.append(" AS " + viewDefStmt_.toSql());
return sb.toString();
}
http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
index 6cdec98..5bb372d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
@@ -30,6 +30,7 @@ import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
+import com.google.common.base.Joiner;
/**
* Base class for CREATE VIEW and ALTER VIEW AS SELECT statements.
@@ -206,6 +207,19 @@ public abstract class CreateOrAlterViewStmtBase extends StatementBase {
return owner_;
}
+ /**
+ * Returns the column names in columnDefs_. Should only be called for non-null
+ * columnDefs_.
+ */
+ protected String getColumnNames() {
+ Preconditions.checkNotNull(columnDefs_);
+ List<String> columnNames = Lists.newArrayList();
+ for (ColumnDef colDef : columnDefs_) {
+ columnNames.add(colDef.getColName());
+ }
+ return Joiner.on(", ").join(columnNames);
+ }
+
public boolean getIfNotExists() { return ifNotExists_; }
public String getInlineViewDef() { return inlineViewDef_; }
public String getTbl() { return tableName_.getTbl(); }
http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
index 1a17aa8..89b6278 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
@@ -26,7 +26,6 @@ import org.apache.impala.thrift.TAccessEvent;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.service.BackendConfig;
-import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
/**
@@ -72,7 +71,7 @@ public class CreateViewStmt extends CreateOrAlterViewStmtBase {
if (ifNotExists_) sb.append("IF NOT EXISTS ");
if (tableName_.getDb() != null) sb.append(tableName_.getDb() + ".");
sb.append(tableName_.getTbl());
- if (columnDefs_ != null) sb.append("(" + Joiner.on(", ").join(columnDefs_) + ")");
+ if (columnDefs_ != null) sb.append("(" + getColumnNames() + ")");
sb.append(" AS ");
sb.append(viewDefStmt_.toSql());
return sb.toString();
http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
index 0a073eb..7209364 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
@@ -363,6 +363,71 @@ public class ToSqlTest extends FrontendTestBase {
}
@Test
+ public void TestCreateView() throws AnalysisException {
+ testToSql(
+ "create view foo_new as select int_col, string_col from functional.alltypes",
+ "default",
+ "CREATE VIEW foo_new AS SELECT int_col, string_col FROM functional.alltypes");
+ testToSql("create view if not exists foo as select * from functional.alltypes",
+ "default", "CREATE VIEW IF NOT EXISTS foo AS SELECT * FROM functional.alltypes");
+ testToSql("create view functional.foo (a, b) as select int_col x, double_col y " +
+ "from functional.alltypes", "default",
+ "CREATE VIEW functional.foo(a, b) AS SELECT int_col x, double_col y " +
+ "FROM functional.alltypes");
+ testToSql("create view foo (aaa, bbb) as select * from functional.complex_view",
+ "default", "CREATE VIEW foo(aaa, bbb) AS SELECT * FROM functional.complex_view");
+ testToSql("create view foo as select trim('abc'), 17 * 7", "default",
+ "CREATE VIEW foo AS SELECT trim('abc'), 17 * 7");
+ testToSql("create view foo (cnt) as " +
+ "select count(distinct x.int_col) from functional.alltypessmall x " +
+ "inner join functional.alltypessmall y on (x.id = y.id) group by x.bigint_col",
+ "default", "CREATE VIEW foo(cnt) AS "+
+ "SELECT count(DISTINCT x.int_col) FROM functional.alltypessmall x " +
+ "INNER JOIN functional.alltypessmall y ON (x.id = y.id) GROUP BY x.bigint_col");
+ testToSql("create view foo (a, b) as values(1, 'a'), (2, 'b')", "default",
+ "CREATE VIEW foo(a, b) AS VALUES((1, 'a'), (2, 'b'))");
+ testToSql("create view foo (a, b) as select 1, 'a' union all select 2, 'b'",
+ "default", "CREATE VIEW foo(a, b) AS SELECT 1, 'a' UNION ALL SELECT 2, 'b'");
+ testToSql("create view test_view_with_subquery as " +
+ "select * from functional.alltypestiny t where exists " +
+ "(select * from functional.alltypessmall s where s.id = t.id)", "default",
+ "CREATE VIEW test_view_with_subquery AS " +
+ "SELECT * FROM functional.alltypestiny t WHERE EXISTS " +
+ "(SELECT * FROM functional.alltypessmall s WHERE s.id = t.id)");
+ }
+
+ @Test
+ public void TestAlterView() throws AnalysisException{
+ testToSql("alter view functional.alltypes_view as " +
+ "select * from functional.alltypesagg", "default",
+ "ALTER VIEW functional.alltypes_view AS SELECT * FROM functional.alltypesagg");
+ testToSql("alter view functional.alltypes_view (a, b) as " +
+ "select int_col, string_col from functional.alltypes", "default",
+ "ALTER VIEW functional.alltypes_view(a, b) AS " +
+ "SELECT int_col, string_col FROM functional.alltypes");
+ testToSql("alter view functional.alltypes_view (a, b) as " +
+ "select int_col x, string_col y from functional.alltypes", "default",
+ "ALTER VIEW functional.alltypes_view(a, b) AS " +
+ "SELECT int_col x, string_col y FROM functional.alltypes");
+ testToSql("alter view functional.alltypes_view as select trim('abc'), 17 * 7",
+ "default", "ALTER VIEW functional.alltypes_view AS SELECT trim('abc'), 17 * 7");
+ testToSql("alter view functional.alltypes_view (aaa, bbb) as " +
+ "select * from functional.complex_view", "default",
+ "ALTER VIEW functional.alltypes_view(aaa, bbb) AS " +
+ "SELECT * FROM functional.complex_view");
+ testToSql("alter view functional.complex_view (abc, xyz) as " +
+ "select year, month from functional.alltypes_view", "default",
+ "ALTER VIEW functional.complex_view(abc, xyz) AS " +
+ "SELECT year, month FROM functional.alltypes_view");
+ testToSql("alter view functional.alltypes_view (cnt) as " +
+ "select count(distinct x.int_col) from functional.alltypessmall x " +
+ "inner join functional.alltypessmall y on (x.id = y.id) group by x.bigint_col",
+ "default", "ALTER VIEW functional.alltypes_view(cnt) AS "+
+ "SELECT count(DISTINCT x.int_col) FROM functional.alltypessmall x " +
+ "INNER JOIN functional.alltypessmall y ON (x.id = y.id) GROUP BY x.bigint_col");
+ }
+
+ @Test
public void TestTableAliases() throws AnalysisException {
String[] tables = new String[] { "alltypes", "alltypes_view" };
String[] columns = new String[] { "int_col", "*" };
[2/4] impala git commit: IMPALA-7254: Inconsistent decimal behavior
for IN/BETWEEN predicate
Posted by ta...@apache.org.
IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
In decimal v2, performing a cast that can result in a loss of precision
is considered as an error. In the prior code when finding a compatible
type for performing a cast between expressions that have decimal and
floating types can cause the decimal expressions to never be compared to
other decimal expressions. This results in a cast to a type that may not
be truly compatible in all expressions, especially when decimal v2 is
enabled. This patch fixes it by grouping all the decimals together to make
sure they are compatible with each other.
Testing:
- Added new FE tests
- Ran all core tests
Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Reviewed-on: http://gerrit.cloudera.org:8080/10882
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/f1643662
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/f1643662
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/f1643662
Branch: refs/heads/master
Commit: f1643662882b73dcc9c32b49ada69f5bfda06770
Parents: 9abd1fe
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Fri Jul 6 00:00:34 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 02:32:09 2018 +0000
----------------------------------------------------------------------
.../org/apache/impala/analysis/Analyzer.java | 38 ++++++++++--
.../impala/analysis/AnalyzeExprsTest.java | 64 ++++++++++++++++++++
2 files changed, 97 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/f1643662/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index 95eec08..0dd68e9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -19,6 +19,7 @@ package org.apache.impala.analysis;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
@@ -2264,13 +2265,40 @@ public class Analyzer {
* returns compatible type otherwise.
*/
public Type castAllToCompatibleType(List<Expr> exprs) throws AnalysisException {
- // Determine compatible type of exprs.
- Expr lastCompatibleExpr = exprs.get(0);
+ // Group all decimal types together so that when finding a compatible type all
+ // decimal types will be compared against each other first. For example:
+ //
+ // These expressions are not compatible because of strict decimal and should throw
+ // an AnalysisException.
+ // [decimal(38, 1), double, decimal(38, 37)]
+ //
+ // Without sorting:
+ // 1. Comparing decimal(38, 1) and double - OK
+ // 2. Comparing double and decimal(38, 37) - OK
+ //
+ // With sorting:
+ // [decimal(38, 1), decimal(38, 37), double]
+ // 1. Comparing decimal(38, 1) with decimal(38, 37) - ERROR
+ //
+ // We need to create a new sorted list instead of mutating it when sorting it because
+ // mutating the original exprs will change the order of the original exprs.
+ List<Expr> sortedExprs = new ArrayList<>(exprs);
+ Collections.sort(sortedExprs, new Comparator<Expr>() {
+ @Override
+ public int compare(Expr expr1, Expr expr2) {
+ if ((expr1.getType().isDecimal() && expr2.getType().isDecimal()) ||
+ (!expr1.getType().isDecimal() && !expr2.getType().isDecimal())) {
+ return 0;
+ }
+ return expr1.getType().isDecimal() ? -1 : 1;
+ }
+ });
+ Expr lastCompatibleExpr = sortedExprs.get(0);
Type compatibleType = null;
- for (int i = 0; i < exprs.size(); ++i) {
- exprs.get(i).analyze(this);
+ for (int i = 0; i < sortedExprs.size(); ++i) {
+ sortedExprs.get(i).analyze(this);
compatibleType = getCompatibleType(compatibleType, lastCompatibleExpr,
- exprs.get(i));
+ sortedExprs.get(i));
}
// Add implicit casts if necessary.
for (int i = 0; i < exprs.size(); ++i) {
http://git-wip-us.apache.org/repos/asf/impala/blob/f1643662/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index a30d98e..76f979c 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -2443,6 +2443,70 @@ public class AnalyzeExprsTest extends AnalyzerTest {
AnalysisError("select " + decimal_5_5 + " > 'cast(1 as timestamp)'",
"operands of type DECIMAL(5,5) and STRING are not comparable: "
+ "CAST(1 AS DECIMAL(5,5)) > 'cast(1 as timestamp)'");
+
+ // IMPALA-7254: Inconsistent decimal behavior when finding a compatible type.
+ // for casting.
+ // In predicate that does not result in a new precision.
+ AnalyzesOk("select cast(2 as double) in " +
+ "(cast(2 as decimal(38, 37)), cast(3 as decimal(38, 37)))");
+ AnalyzesOk("select cast(2 as decimal(38, 37)) in " +
+ "(cast(2 as double), cast(3 as decimal(38, 37)))");
+ AnalyzesOk("select cast(2 as decimal(38, 37)) in " +
+ "(cast(2 as decimal(38, 37)), cast(3 as double))");
+ // In predicate that results in a precision.
+ AnalyzesOk("select cast(2 as double) in " +
+ "(cast(2 as decimal(5, 3)), cast(3 as decimal(10, 5)))");
+ AnalyzesOk("select cast(2 as decimal(5, 3)) in " +
+ "(cast(2 as double), cast(3 as decimal(10, 5)))");
+ AnalyzesOk("select cast(2 as decimal(5, 3)) in " +
+ "(cast(2 as decimal(10, 5)), cast(3 as double))");
+ // In predicate that results in a loss of precision.
+ AnalysisError("select cast(2 as decimal(38, 37)) in (cast(2 as decimal(38, 1)))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(2 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as double) in " +
+ "(cast(2 as decimal(38, 37)), cast(3 as decimal(38, 1)))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(3 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as decimal(38, 37)) in " +
+ "(cast(2 as double), cast(3 as decimal(38, 1)))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(3 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as decimal(38, 37)) in " +
+ "(cast(2 as decimal(38, 1)), cast(3 as double))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(2 AS DECIMAL(38,1))'");
+ // Between predicate that does not result in a new precision.
+ AnalyzesOk("select cast(2 as double) between " +
+ "cast(2 as decimal(38, 37)) and cast(3 as decimal(38, 37))");
+ AnalyzesOk("select cast(2 as decimal(38, 37)) between " +
+ "cast(2 as double) and cast(3 as decimal(38, 37))");
+ AnalyzesOk("select cast(2 as decimal(38, 37)) between " +
+ "cast(2 as decimal(38, 37)) and cast(3 as double)");
+ // Between predicate that results in a new precision.
+ AnalyzesOk("select cast(2 as double) between " +
+ "cast(2 as decimal(5, 3)) and cast(3 as decimal(10, 5))");
+ AnalyzesOk("select cast(2 as decimal(5, 3)) between " +
+ "cast(2 as double) and cast(3 as decimal(10, 5))");
+ AnalyzesOk("select cast(2 as decimal(5, 3)) between " +
+ "cast(2 as decimal(10, 5)) and cast(3 as double)");
+ // Between predicate that results in a loss of precision.
+ AnalysisError("select cast(2 as decimal(38, 37)) between " +
+ "cast(2 as decimal(38, 1)) and cast(3 as decimal(38, 3))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(2 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as double) between " +
+ "cast(2 as decimal(38, 37)) and cast(3 as decimal(38, 1))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(3 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as decimal(38, 37)) between " +
+ "cast(2 as double) and cast(3 as decimal(38, 1))",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(3 AS DECIMAL(38,1))'");
+ AnalysisError("select cast(2 as decimal(38, 37)) between " +
+ "cast(2 as decimal(38, 1)) and cast(3 as double)",
+ "Incompatible return types 'DECIMAL(38,37)' and 'DECIMAL(38,1)' of exprs " +
+ "'CAST(2 AS DECIMAL(38,37))' and 'CAST(2 AS DECIMAL(38,1))'");
}
@Test
[4/4] impala git commit: IMPALA-7260: Fix decimal binary predicates
Posted by ta...@apache.org.
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Reviewed-on: http://gerrit.cloudera.org:8080/10888
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/36ce54bd
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/36ce54bd
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/36ce54bd
Branch: refs/heads/master
Commit: 36ce54bd5c5aca2a09294460dcc1c2e5a80ba497
Parents: c465150
Author: Taras Bobrovytsky <ta...@apache.org>
Authored: Fri Jul 6 18:16:02 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 05:09:38 2018 +0000
----------------------------------------------------------------------
.../java/org/apache/impala/analysis/Expr.java | 19 ++++++++++++++-----
.../apache/impala/analysis/AnalyzeExprsTest.java | 9 +++++++++
2 files changed, 23 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/36ce54bd/fe/src/main/java/org/apache/impala/analysis/Expr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index e85a4a4..e34a58c 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -26,7 +26,6 @@ import java.util.ListIterator;
import java.util.Set;
import org.apache.impala.analysis.BinaryPredicate.Operator;
-import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Function.CompareMode;
import org.apache.impala.catalog.ImpaladCatalog;
@@ -470,10 +469,20 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
int ix = Math.min(fnArgs.length - 1, i);
if (fnArgs[ix].isWildcardDecimal()) {
if (children_.get(i).type_.isDecimal() && ignoreWildcardDecimals) continue;
- Preconditions.checkState(resolvedWildcardType != null);
- Preconditions.checkState(!resolvedWildcardType.isInvalid());
- if (!children_.get(i).type_.equals(resolvedWildcardType)) {
- castChild(resolvedWildcardType, i);
+ if (children_.get(i).type_.isDecimal() || !ignoreWildcardDecimals) {
+ Preconditions.checkState(resolvedWildcardType != null);
+ Preconditions.checkState(!resolvedWildcardType.isInvalid());
+ if (!children_.get(i).type_.equals(resolvedWildcardType)) {
+ castChild(resolvedWildcardType, i);
+ }
+ } else if (children_.get(i).type_.isNull()) {
+ castChild(ScalarType.createDecimalType(), i);
+ } else {
+ Preconditions.checkState(children_.get(i).type_.isScalarType());
+ // It is safe to assign an arbitrary decimal here only if the backend function
+ // can handle it (in which case ignoreWildcardDecimals is true).
+ Preconditions.checkState(ignoreWildcardDecimals);
+ castChild(((ScalarType) children_.get(i).type_).getMinResolutionDecimal(), i);
}
} else if (!children_.get(i).type_.matchesType(fnArgs[ix])) {
castChild(fnArgs[ix], i);
http://git-wip-us.apache.org/repos/asf/impala/blob/36ce54bd/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index 76f979c..df0c4e7 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -262,6 +262,15 @@ public class AnalyzeExprsTest extends AnalyzerTest {
}
}
+ // IMPALA-7260: Check that when a numerical type gets implicitly cast to decimal,
+ // we do not get an error when the type of the decimal operand is not able to fit
+ // all of the digits of the non-deicmal operand.
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as bigint)");
+ AnalyzesOk("select cast(1 as bigint) " + operator + " cast(2 as decimal(38,37))");
+ AnalyzesOk("select cast(1 as decimal(38,1)) " + operator + " cast(2 as bigint)");
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as tinyint)");
+ AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as double)");
+
// Chars of different length are comparable
for (int i = 1; i < 16; ++i) {
AnalyzesOk("select cast('hi' as char(" + i + ")) " + operator +