[
https://issues.apache.org/jira/browse/KYLIN-1122?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15203058#comment-15203058
]
liyang edited comment on KYLIN-1122 at 3/20/16 3:04 AM:
--------------------------------------------------------
[~wangxiaoyu], sorry for the delay, finally got some time to review.
For KYLIN-1122-B2, it's great work! I think it's ready to merge into master
given IT pass. Just one minor comment.
- For readability, I'm thinking if {{SQLDigest.isRawQuery}} can be decided in
the constructor of SQLDigest by simply checking {{groupbyColumns}} alone.
Currently isRawQuery is assigned in {{OLAPEnumerator.hackNoGroupByAggregation}}
and is checking both {{groupbyColumns}} and {{metricColumns}}. This is a bit
confusing for two reasons. First, {{RawMeasureType.influenceCapabilityCheck}}
also checks if raw query but in a slightly different way, ideally it should
reference the isRawQuery on the digest parameter so we have a consistent logic
everywhere. Second, {{metricColumns}} is updated in a few places in the code,
this makes reader to doubt if {{isRawQuery}} should be updated as well. It's
better to let isRawQuery derive from {{groupbyColumns}} only, to be clear that
it's an immutable property depends only on the SQL input.
Thanks again for the great work. Feel free to merge on your own. :-)
was (Author: liyang.g...@gmail.com):
[~wangxiaoyu], sorry for the delay, finally got some time to review.
For KYLIN-1122-B2, it's great work! I think it's ready to merge into master
given IT pass. Just one minor comment.
- For readability, I'm thinking if {{SQLDigest.isRawQuery}} can be decided in
the constructor of SQLDigest by simply checking {{groupbyColumns}} alone.
Currently isRawQuery is assigned in {{OLAPEnumerator.hackNoGroupByAggregation}}
and is checking both {{groupbyColumns}} and {{metricColumns}}. This is a bit
confusing for two reasons. First, {{RawMeasureType.influenceCapabilityCheck}}
also checks if raw query but in a slightly different way, ideally it should
reference the isRawQuery on the digest parameter so we have a consistent logic
everywhere. Second, {{metricColumns}} is updated in a few places in the code,
this makes reader to doubt if {{isRawQuery}} should be updated as well. It's
better to let isRawQuery derive from {{groupbyColumns}} only, to be clear that
it's an immutable property depends only on the SQL input.
> Kylin support detail data query from fact table
> -----------------------------------------------
>
> Key: KYLIN-1122
> URL: https://issues.apache.org/jira/browse/KYLIN-1122
> Project: Kylin
> Issue Type: New Feature
> Components: Query Engine
> Affects Versions: v1.2
> Reporter: Xiaoyu Wang
> Assignee: liyang
> Fix For: Backlog
>
> Attachments:
> 0001-KYLIN-1122-Kylin-support-detail-data-query-from-fact(2.x-staging).patch,
> 0001-KYLIN-1122-Kylin-support-detail-data-query-from-fact(update-v2-1.x-staging).patch,
>
> 0001-KYLIN-1122-Kylin-support-detail-data-query-from-fact-new-impl-under-refactoring-2.x-staging.patch
>
>
> Now Kylin does not support query correct detail rows from fact table like:
> select column1,column2,column3 from fact_table
> The jira KYLIN-1075 add the "SUM" function on the measure column if defined.
> But only the column number type is support.
> I change some code to support this issue:
> Add a "VALUE" measure function : the same value and datatype in the input and
> output of this function.
> If you want to query detail data from fact table
> *require*:
> 1.Configure the column which not dimensions to "VALUE" or "SUM" measure.(If
> not configure measure function in the column will get NULL value)
> 2.The source table must has an unique value column and configure it as
> dimension.
> If you have the better solution please comment here.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
