[jira] [Commented] (KYLIN-3072) Fix the regular expression in function 'removeCommentInSql'

[peng.jianhua (JIRA)]

[ 
https://issues.apache.org/jira/browse/KYLIN-3072?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274001#comment-16274001
 ] 

peng.jianhua commented on KYLIN-3072:
-

Hi [~Shaofengshi], I'm sorry. I did not describe this issue clearly. If you 
still think there is no problem, I will close the issue according to your 
review. The essence of this problem is that the logic of parsing is flawed. The 
current logic is as following:
1. Firstly,  the "--" was parsed.
2. Secondly, the "/* */" was parsed.
Based on the above parsing logic, once the "/* */" contains the "--" and the 
"*/" and the "--" are in the same line , the parsing will be wrong. Because the 
"*/" will be removed when the program parses the "--". When parsing the "/* */" 
again, the program parses fail because the "*/" was removed.

What we modify is the wrong logic of the program. Based on the above logic 
modification, all possible combinations will not be a problem, do you think?

> Fix the regular expression in function 'removeCommentInSql'
> ---
>
> Key: KYLIN-3072
> URL: https://issues.apache.org/jira/browse/KYLIN-3072
> Project: Kylin
>  Issue Type: Bug
>  Components: Query Engine
>Reporter: peng.jianhua
>Assignee: peng.jianhua
> Attachments: 
> 0001-KYLIN-3072-Fix-the-regular-expression-in-function-re.patch, 01.png, 
> 02.PNG
>
>
> select * from kylin_sales /\*comments--comments*/
> we should remove comments like '/\*comments*/' first,then remove comments 
> like '--comments' , or the sql above will have an exception.
> please refer to 01.png.
> if we exchange the sequence of this two comment patterns,we will get 02.png.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (KYLIN-3072) Fix the regular expression in function 'removeCommentInSql'

[Shaofeng SHI (JIRA)]

[ 
https://issues.apache.org/jira/browse/KYLIN-3072?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273799#comment-16273799
 ] 

Shaofeng SHI commented on KYLIN-3072:
-

I know it; My question is, do you see a case that such comment style generated 
from a BI tool like Tableau, Cognos or MSTR? or it is made by manual? 

If by manual, there are many possible combinations, can this patch solve all of 
them? Besides, Calcite can recognize comments, Kylin doesn't need take over 
that too much.

I don't think this is a solid enhancement that worth a JIRA to discuss on it. 
If a strange comment couldn't be recognized and got error, then please correct 
it, that's it.

> Fix the regular expression in function 'removeCommentInSql'
> ---
>
> Key: KYLIN-3072
> URL: https://issues.apache.org/jira/browse/KYLIN-3072
> Project: Kylin
>  Issue Type: Bug
>  Components: Query Engine
>Reporter: peng.jianhua
>Assignee: peng.jianhua
> Attachments: 
> 0001-KYLIN-3072-Fix-the-regular-expression-in-function-re.patch, 01.png, 
> 02.PNG
>
>
> select * from kylin_sales /\*comments--comments*/
> we should remove comments like '/\*comments*/' first,then remove comments 
> like '--comments' , or the sql above will have an exception.
> please refer to 01.png.
> if we exchange the sequence of this two comment patterns,we will get 02.png.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)