Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testutil: When there is // in the json test data, the test framework will report an error. #26249

Open
LittleFall opened this issue Jul 14, 2021 · 5 comments
Labels
sig/sql-infra SIG: SQL Infra type/enhancement The issue or PR belongs to an enhancement.

Comments

@LittleFall
Copy link
Contributor

LittleFall commented Jul 14, 2021

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

change any testdata json file , add // in it, just like:

      {
        "SQL": "EXPLAIN SELECT * from t join s; -- FIXME: there is some bug on it, see https://github.com/pingcap/tidb/issues/26249",
        "Plan": [
          "HashJoin_8 100000000.00 root  CARTESIAN inner join",
          "├─TableReader_17(Build) 10000.00 root  data:TableFullScan_16",
          "│ └─TableFullScan_16 10000.00 cop[tiflash] table:s keep order:false, stats:pseudo",
          "└─TableReader_13(Probe) 10000.00 root  data:TableFullScan_12",
          "  └─TableFullScan_12 10000.00 cop[tiflash] table:t keep order:false, stats:pseudo"
        ],
        "Warn": [
          "MPP mode may be blocked because `Cartesian Product` is only supported by broadcast join, check value and documents of variables `tidb_opt_broadcast_cartesian_join`, `tidb_broadcast_join_threshold_size` and `tidb_broadcast_join_threshold_count`.",
          "MPP mode may be blocked because `Cartesian Product` is only supported by broadcast join, check value and documents of variables `tidb_opt_broadcast_cartesian_join`, `tidb_broadcast_join_threshold_size` and `tidb_broadcast_join_threshold_count`."
        ]
      },

add run this test:

/usr/local/Cellar/go/1.16.3/libexec/bin/go test -v ./planner/core/... -check.f TestEnforceMPP.* -check.vv

2. What did you expect to see? (Required)

test pass.

3. What did you see instead (Required)

c.Assert(err, IsNil)
Expected :
Actual   :*json.SyntaxError = &json.SyntaxError{msg:"invalid character 'P' after object key:value pair", Offset:30264} ("invalid character 'P' after object key:value pair")

4. What is your TiDB version? (Required)

20210714 master

https://github.com/pingcap/tidb/tree/737fa5b743c365e6bd7a7c30c54f9cb299b01e8d

@LittleFall LittleFall added the type/bug The issue is confirmed as a bug. label Jul 14, 2021
@LittleFall
Copy link
Contributor Author

LittleFall commented Jul 14, 2021

this may because tidb wants to ignore the comment in json.

@LittleFall
Copy link
Contributor Author

It seems that this is not a planner issue? I take the test in the planner as an example, but other tests may also have this problem.

@jyz0309
Copy link
Contributor

jyz0309 commented Jul 19, 2021

I have checked the code, and seem it is the normal behavior?

// Remove comments, since they are not allowed in json.

@LittleFall
Copy link
Contributor Author

I have checked the code, and seem it is the normal behavior?

// Remove comments, since they are not allowed in json.

I have seen it too, but I think it is unreasonable, tidb introduced a bug (we can't type // in json) for an unnecessary feature (support comments in json).

Or maybe we can find a way to support them both, such as a better parser.

@yudongusa
Copy link

transfer this ticket to parser/sql_infra team for further enhancement.

@yudongusa yudongusa added sig/sql-infra SIG: SQL Infra and removed sig/planner SIG: Planner labels Aug 24, 2021
@bb7133 bb7133 added type/enhancement The issue or PR belongs to an enhancement. and removed type/bug The issue is confirmed as a bug. labels Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

6 participants