Mailing-List: contact dev-help@phoenix.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@phoenix.apache.org
Date: Sat, 21 May 2016 01:45:12 +0000 (UTC)
From: "James Taylor (JIRA)" <jira@apache.org>
To: dev@phoenix.incubator.apache.org
Message-ID: <JIRA.12967181.1462910379000.256089.1463795112995@Atlassian.JIRA>
In-Reply-To: <JIRA.12967181.1462910379000@Atlassian.JIRA>
References: <JIRA.12967181.1462910379000@Atlassian.JIRA> <JIRA.12967181.1462910379155@arcas>
Subject: [jira] [Commented] (PHOENIX-2886) Union ALL with Char column  not
 present in the table in Query 1 but in Query 2 throw exception
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Sat, 21 May 2016 01:45:17 -0000


    [ 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)