support oceanbase #429

bashaojing · 2021-12-15T03:55:23Z

No description provided.

oraluben · 2021-12-15T05:56:49Z

Do you intend to add a full test workflow for OceanBase? (install OceanBase, run test, etc.)

mrigger

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.

mrigger · 2021-12-15T21:01:16Z

.travis.yml

        - sleep 5
      script:
        - CLICKHOUSE_AVAILABLE=true mvn -Dtest=ClickHouseBinaryComparisonOperationTest test
+    - name: OceanBase


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.

mrigger · 2021-12-15T21:01:53Z

src/sqlancer/oceanbase/OceanBaseErrors.java

+        errors.add("Invalid numeric");
+
+
+        if (true) {


I guess we can remove the if and only keep its body?

mrigger · 2021-12-15T21:05:02Z

src/sqlancer/oceanbase/README.md

@@ -0,0 +1,21 @@
+## Install Oceanbase


That's very useful! Thanks for adding also a README.md.

mrigger · 2021-12-15T21:07:00Z

src/sqlancer/oceanbase/ast/OceanBaseConstant.java

+                try {
+                    char currentChar = value.charAt(i-1);
+                    int currentVal= Integer.valueOf(currentChar);
+                    if (currentVal < 48||currentVal > 57)


Having named constants for 48 and 57 would be useful (but I assume that one of the style checks will complain about this anyway).

mrigger · 2021-12-15T21:14:53Z

src/sqlancer/oceanbase/oracle/OceanBaseNoRECOracle.java

+
+        OceanBaseExpression expr = OceanBaseConstant.createNullConstant(); 
+        switch(i){
+           case 0:        


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)?

mrigger · 2021-12-15T21:17:35Z

src/sqlancer/oceanbase/oracle/OceanBaseNoRECOracle.java

+        return firstCount;
+    }
+
+    private OceanBaseExpression getTrueExpr(OceanBaseExpression randomWhereCondition){


Perhaps you can add a short comment explaining how this differs from the standard NoREC version?

mrigger

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?

bashaojing · 2021-12-16T13:21:05Z

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?

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.

mrigger · 2021-12-16T21:55:27Z

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.

mrigger · 2021-12-21T22:18:52Z

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?

support oceanbase

3f29b04

fix for hint,update default port,add readme for install oceanbase

6b7c6f6

mrigger reviewed Dec 15, 2021

View reviewed changes

fix for format check and some comments

9b02f13

mrigger approved these changes Dec 16, 2021

View reviewed changes

mrigger merged commit 734b984 into sqlancer:master Dec 16, 2021

PsiACE mentioned this pull request Jan 12, 2022

[test] add sqlancer test databendlabs/databend#1768

Closed

support oceanbase #429

support oceanbase #429

Uh oh!

Conversation

bashaojing commented Dec 15, 2021

Uh oh!

oraluben commented Dec 15, 2021

Uh oh!

mrigger left a comment

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

mrigger left a comment

Choose a reason for hiding this comment

Uh oh!

bashaojing commented Dec 16, 2021

Uh oh!

mrigger commented Dec 16, 2021

Uh oh!

mrigger commented Dec 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants