Automatic Query Fixer: init the project and implement a query token service #73
Conversation
create and test a QueryTokenService
mingen-pan
changed the title
|
|
||
| compileOnly 'org.projectlombok:lombok:1.18.12' | ||
| annotationProcessor 'org.projectlombok:lombok:1.18.12' | ||
| implementation 'org.apache.calcite.avatica:avatica-core:1.17.0' |
There was a problem hiding this comment.
We still decided to use this :)?
There was a problem hiding this comment.
Good point. It seems some place can be replaced by AutoValue, but not every class acts as an immutable value and some may need to be modified (by setter) after creation. Thus, I think lombok may be better in some situations. If AutoValue can do the similar job, I will change to it.
There was a problem hiding this comment.
Autovalue builder pattern can help you generate getters/setters.
https://github.com/google/auto/blob/master/value/userguide/builders.md
We are open to use lib we want not sure if one is significant better than the other given the current use case.
| public static void main(String[] args) { | ||
|
|
||
| if (args.length == 0 ) { | ||
| System.out.println("not enough arguments"); |
There was a problem hiding this comment.
We should use logger
https://github.com/google/flogger
or java logger https://docs.oracle.com/javase/7/docs/api/java/util/logging/Logger.html
There was a problem hiding this comment.
| @@ -0,0 +1,14 @@ | |||
| package com.google.cloud.bigquery.utils.auto_query_fixer; | |||
|
|
|||
| public class Application { | |||
There was a problem hiding this comment.
I don't quite like "Application"
I saw many people use "'MyClass'Main" or similar
maybe 'QueryFixerMain'?
| return; | ||
| } | ||
|
|
||
| String query = args[0]; |
There was a problem hiding this comment.
we never use this? Add a TODO?
There was a problem hiding this comment.
https://github.com/google/google-java-format maybe we can install this :)
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class QueryTokenService { |
There was a problem hiding this comment.
Please add comment for the class and each method according to the style guide. It would be harder to understand without comments :).
| List<IToken> tokens = new ArrayList<>(); | ||
|
|
||
| // a parser contains token manager, which will be used to tokenize sql. | ||
| final SqlParser.ConfigBuilder configBuilder = |
There was a problem hiding this comment.
Should this be a class member iso it can be used in other places too?
There was a problem hiding this comment.
Move this to the new class BigQueryParserFactory
| SqlBabelParserImpl parserImpl = (SqlBabelParserImpl) configBuilder.build().parserFactory().getParser(new SourceStringReader(sql)); | ||
|
|
||
| Token token; | ||
| while ((token = parserImpl.getNextToken()).kind != 0) { |
There was a problem hiding this comment.
Curious what is 'kind' here? why we can use this as termination condition?
There was a problem hiding this comment.
| String[] lines = sql.split("\n"); | ||
| validateToken(lines, token); | ||
|
|
||
| // the token's line and column number are 1-index, |
There was a problem hiding this comment.
nit: Comments should start with Upper case, // 'The' token's line and ...
There was a problem hiding this comment.
nit: Looks like the comment line is not well formatted, you can use the java formatter to help format it better: ).
There was a problem hiding this comment.
| } | ||
| } | ||
|
|
||
| // end index is excluded! |
There was a problem hiding this comment.
nit: Please remove random comment or make it more readable :)
There was a problem hiding this comment.
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public class Choice { |
There was a problem hiding this comment.
Please add comment for each class.
Add comments
add documents for classes
init a project with basic structures
create and test the QueryTokenService