support oceanbase - #429
Conversation
Sorry, something went wrong.
|
Do you intend to add a full test workflow for OceanBase? (install OceanBase, run test, etc.) |
Sorry, something went wrong.
mrigger
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR! I think we can merge it soon. I added some minor comments. I would appreciate it if you would address them before we merge the PR. Currently, also some style checks are failing. You can check locally using mvn verify.
Sorry, something went wrong.
| - sleep 5 | ||
| script: | ||
| - CLICKHOUSE_AVAILABLE=true mvn -Dtest=ClickHouseBinaryComparisonOperationTest test | ||
| - name: OceanBase |
There was a problem hiding this comment.
We are now actually using GitHub Actions for testing. For example, see https://github.com/sqlancer/sqlancer/blob/master/.github/workflows/main.yml#L187 for how the MySQL tests are executed in the CI. If you want, you can also add this in a follow-up PR.
Sorry, something went wrong.
| errors.add("Invalid numeric"); | ||
|
|
||
|
|
||
| if (true) { |
There was a problem hiding this comment.
I guess we can remove the if and only keep its body?
Sorry, something went wrong.
| @@ -0,0 +1,21 @@ | |||
| ## Install Oceanbase | |||
There was a problem hiding this comment.
That's very useful! Thanks for adding also a README.md.
Sorry, something went wrong.
| try { | ||
| char currentChar = value.charAt(i-1); | ||
| int currentVal= Integer.valueOf(currentChar); | ||
| if (currentVal < 48||currentVal > 57) |
There was a problem hiding this comment.
Having named constants for 48 and 57 would be useful (but I assume that one of the style checks will complain about this anyway).
Sorry, something went wrong.
|
|
||
| OceanBaseExpression expr = OceanBaseConstant.createNullConstant(); | ||
| switch(i){ | ||
| case 0: |
There was a problem hiding this comment.
The hard-coded constants look quite error-prone. Perhaps you can add an enum instead, of which a random value is selected, and based on which you perform the switch (or where each enum value implements a method that you call)?
Sorry, something went wrong.
| return firstCount; | ||
| } | ||
|
|
||
| private OceanBaseExpression getTrueExpr(OceanBaseExpression randomWhereCondition){ |
There was a problem hiding this comment.
Perhaps you can add a short comment explaining how this differs from the standard NoREC version?
Sorry, something went wrong.
mrigger
left a comment
There was a problem hiding this comment.
LGTM! Thanks a lot again for the PR! It would be ideal to contribute a GitHub Actions workflow to test the OceanBase implementation to ensure that no future changes break it. Do you plan to also contribute such a workflow?
Sorry, something went wrong.
Thanks for you approval! It needs some preceding conditions for installing oceanbase. I'm not sure installing oceanbase will always success when running ci and block other commit. |
Sorry, something went wrong.
|
If the installation would work on the current version, I think it would be good enough. We regularly adapt to updates to database systems. For example, H2 currently fails due to an update, which I hope to address soon. |
Sorry, something went wrong.
|
Another issue: I noticed that building SQLancer now results in multiple warnings: sqlancer/src/sqlancer/oceanbase/ast/OceanBaseConstant.java:[96,24] The constructor java.lang.Double(double) is deprecated
[WARNING] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseConstant.java:[107,34] The constructor java.lang.Double(double) is deprecated
[WARNING] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseConstant.java:[113,34] The constructor java.lang.Double(double) is deprecated
[WARNING] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseConstant.java:[281,54] Dead code
[WARNING] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseConstant.java:[305,50] Dead code
[WARNING] sqlancer/src/sqlancer/oceanbase/gen/datadef/OceanBaseIndexGenerator.java:[85,18] The type sqlancer.oceanbase.gen.datadef.OceanBaseIndexGenerator.PartitionOptions is never used locally
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[33,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[47,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[67,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[84,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[98,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[INFO] sqlancer/src/sqlancer/oceanbase/ast/OceanBaseComputableFunction.java:[105,38] Varargs methods should only override or be overridden by other varargs methods unlike new sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction(){}.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression[]) and sqlancer.oceanbase.ast.OceanBaseComputableFunction.OceanBaseFunction.apply(sqlancer.oceanbase.ast.OceanBaseConstant[], sqlancer.oceanbase.ast.OceanBaseExpression...)
[WARNING] sqlancer/src/sqlancer/oceanbase/OceanBaseUserCheckException.java:[3,14] The serializable class OceanBaseUserCheckException does not declare a static final serialVersionUID field of type long
Could you please fix them? |
Sorry, something went wrong.
No description provided.