phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Taylor (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (PHOENIX-2886) Union ALL with Char column not present in the table in Query 1 but in Query 2 throw exception
Date Sat, 21 May 2016 01:45:12 GMT

    [ https://issues.apache.org/jira/browse/PHOENIX-2886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15294632#comment-15294632
] 

James Taylor commented on PHOENIX-2886:
---------------------------------------

Thanks for the patch, [~ayingshu] and the reviews, [~sergey.soldatov]. Here's some feedback:
- You shouldn't need the special case for CoerceExpression here or below, as the data type
and max length of the outermost expression (CoerceExpression or not) is what you're interested
in.
{code}
+            if (pro.getExpression() instanceof CoerceExpression) {
+                Expression expr = ((CoerceExpression)pro.getExpression()).getChild();
+                targetTypes.add(new TargetDataExpression(expr.getDataType(),
+                    pro.getExpression().getMaxLength() == null?
+                            0 : pro.getExpression().getMaxLength()));
+            } else {
+                targetTypes.add(new TargetDataExpression(pro.getExpression().getDataType(),
+                    pro.getExpression().getMaxLength() == null?
+                            0 : pro.getExpression().getMaxLength()));
+            }
{code}
- In addition to keeping track of the max length, you should keep track of the max scale as
well (when it's not null). You should track this in your TargetDataExpression object.
- You need to track sort order across all top level projector expressions and have the coerce
use the common one (if they're all the same), or SortOrder.getDefault() if they're different.
You should track this in your TargetDataExpression object.
- What happens if TargetDataExpression ends up with a 0 for max length? You should be using
a null for max length in the case. Why not make TargetDataExpression.maxLength an Integer
instead and allow it to stay null?
- Same for scale. Add it to TargetDataExpression as an Integer and handle null correctly.
- Need tests for the above - mix of different scales, all varchars (where maxLength would
end up 0), different sort orders.
- Here, when you create the Expression[], just call CoerceExpression.create() as it's a noop
if the type, maxLength, scale, etc. are the same. You don't want to repeat that logic here.
{code}
+    private static TupleProjector getTupleProjector(RowProjector rowProj,
+            List<TargetDataExpression> targetTypes) throws SQLException {
+        Expression[] exprs = new Expression[targetTypes.size()];
+        int i = 0;
+        Expression expr = null;
+        for (ColumnProjector colProj : rowProj.getColumnProjectors()) {
+            if (!colProj.getExpression().getDataType().equals(targetTypes.get(i).getType()))
{
+                expr = CoerceExpression.create(colProj.getExpression(),
+                    targetTypes.get(i).getType(), colProj.getExpression().getSortOrder(),
+                    targetTypes.get(i).getMaxLength());
+                exprs[i] = expr;
+            } else
+                exprs[i] = colProj.getExpression();
+            i++;
+        }
+        return new TupleProjector(exprs);
+    }
{code}
- Minor nit, but use { } here and no need to check for {{targetTypes.get(i).getType() == type}}
as isCoercibleTo handles that already.
{code}
+    private static void compareExperssions(int i, Expression expression, PDataType type,
+            List<TargetDataExpression> targetTypes) throws SQLException {
+        if (targetTypes.get(i).getType() == type ||
+                type.isCoercibleTo(targetTypes.get(i).getType()))
+            ;
+        else if (targetTypes.get(i).getType().isCoercibleTo(type)) {
{code}

> Union ALL with Char column  not present in the table in Query 1 but in Query 2 throw
exception
> ----------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-2886
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-2886
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Alicia Ying Shu
>            Assignee: Alicia Ying Shu
>             Fix For: 4.8.0
>
>         Attachments: PHOENIX-2886-v1.patch, PHOENIX-2886-v2.patch, PHOENIX-2886-v3.patch,
PHOENIX-2886.patch
>
>
> To reproduce:
> create table person ( id bigint not null primary key, firstname char(10), lastname varchar(10)
);
> upsert into person values( 1, 'john', 'doe');
> upsert into person values( 2, 'jane', 'doe');
> -- fixed value for char(10)
> select id, 'foo' firstname, lastname from person union all select * from person;
> java.lang.RuntimeException: java.sql.SQLException: ERROR 201 (22000): Illegal data. Expected
length of at least 106 bytes, but had 13
> -- fixed value for bigint
> select cast( 10 AS bigint) id, 'foo' firstname, lastname from person union all select
* from person;
> java.lang.RuntimeException: java.sql.SQLException: ERROR 201 (22000): Illegal data. Expected
length of at least 106 bytes, but had 13



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message