TuiQiao / CBoard

An easy to use, self-service open BI reporting and BI dashboard platform.
https://tuiqiao.github.io/CBoardDoc/#/
Apache License 2.0
3.04k stars 1.17k forks source link

kylin datasource return wrong result for Date type column #622

Closed lordk911 closed 5 years ago

lordk911 commented 5 years ago

when there is a column with Date type , when I query 2012-06-28's data , after format , it will display as 2012-06-27

I was use cboard branch 0.4.2 and kylin 2.4.0.

I found it was because of two point : one , jdbc url show point what the timezone will use :

jdbc:kylin:timeZone=CST//127.0.0.1:7070/learn_kylin

two,JdbcDataProvider.queryAggData should getDate from resultset:

while (rs.next()) {
                String[] row = new String[columnCount];
                for (int j = 0; j < columnCount; j++) {
                    int columType = metaData.getColumnType(j + 1);
                    switch (columType) {
                    case java.sql.Types.DATE:
                        row[j] = rs.getDate(j + 1).toString();
                        break;
                    default:
                        row[j] = rs.getString(j + 1);
                        break;
                    }

                }
                list.add(row);
            }
lordk911 commented 5 years ago

and after change the jdbc url ,when I deploy war to tomcat 8.5.37 , will get error :

[ERROR][18:45:03][DruidDataSource.init():688] init datasource error, url: jdbc:kylin:timeZone=CST//127.0.0.1:7070/learn_kylin
java.sql.SQLException: java.net.UnknownHostException: jdbc: Name or service not known

change to use the latest version of druid (1.1.12) will be ok.

yzhang921 commented 5 years ago

Thanks for your feedback, could you create a pull request for this issue, please? I think it would be better to fix it by your name.

lordk911 commented 5 years ago

ok, I will have a try ,thanks.

lordk911 commented 5 years ago

for case of use timezone on jdbc url ,kylin jdbc driver need to be update , which version I use is 2.4.0

yzhang921 commented 5 years ago

Sorry, I didn't notice that you are using JDBC data provider to connect KYLIN. Why don't you use native Kylin data provider, which can generate better performance dynamic query on Kylin?

lordk911 commented 5 years ago

Yes , I think if use native Kylin will not meet this issue , and it will query dim parameter from dim table. But there is someting wrong when use native kylin : When I try to get filter condition for a dim , the sql generate was wrong : `[INFO][13:43:59][KylinDataProvider.queryDimVals():152]

SELECT "KYLIN_SALES"."TRANS_ID" 
  FROM DEFAULT.KYLIN_SALES null  
 GROUP BY "KYLIN_SALES"."TRANS_ID" 
 ORDER BY "KYLIN_SALES"."TRANS_ID"
yzhang921 commented 5 years ago

Looks like table alias is not be successfully initialized in kylinModel. Maybe this is one more metadata compatible issue.

  kylinModel.getTableAlias(tableName) // return a null value

https://github.com/TuiQiao/CBoard/blob/fbe49a1373c34255dcd1ba238a0d1cb9207f8da6/src/main/java/org/cboard/kylin/KylinDataProvider.java#L151 .

Could you try to debug from the line I marked above?

lordk911 commented 5 years ago

kylinModel.getTableAlias(tableName) which param: tableName is a full table name, but on line 77 of KylinModel2x.initMetaData() it put table alias name as key :

model.getJSONArray("lookups").stream().map(e -> (JSONObject) e)
                .forEach(s -> tableAlias.put(s.getString("alias"),s.getString("table"))); 
lordk911 commented 5 years ago

one look up table may have multy alias name. and the the table for query and it alias should also be surround with quta .

yzhang921 commented 5 years ago

Could you help fix this issue? It would be helpful to those your kind of company using Kylin as a data source.

lordk911 commented 5 years ago

I have fix it , and commit with the PR . Sorry , because of my ide is eclipse it's default format Indentation is Tab, I have change it to space .

yzhang921 commented 5 years ago

Thanks very much for your work. Looks like you make another mistake. Please don't format the whole file, because it will make the code diff too mess to do code review. Diff View I know it is really complicated to do even simple contribution for a newbie. In order to let others contributor/user easy track code change, we should try our best to keep the code change clean, so you would better re-fork this branch and pick out only the issue code segment and recreate a pull request. Hope your consideration! And I'll back you to complete this PR.