Bug(sink): create tidb sink panic

[huangjw806]

tidb-cdc-sink integration test:
https:/risingwavelabs/risingwave/tree/main/integration_tests/tidb-cdc-sink

run command:

python3 run_demos.py --case tidb-cdc-sink --format json

test error log:

Running SQL file: /home/ubuntu/risingwave/integration_tests/tidb-cdc-sink/create_source.sql
CREATE_TABLE
CREATE_TABLE
Running SQL file: /home/ubuntu/risingwave/integration_tests/tidb-cdc-sink/create_mv.sql
psql:/home/ubuntu/risingwave/integration_tests/tidb-cdc-sink/create_mv.sql:23: NOTICE:
The ORDER BY clause in the CREATE MATERIALIZED VIEW statement does not guarantee that the rows selected out of this materialized view is returned in this order.
It only indicates the physical clustering of the data, which may improve the performance of queries issued against this materialized view.
CREATE_MATERIALIZED_VIEW
psql:/home/ubuntu/risingwave/integration_tests/tidb-cdc-sink/create_mv.sql:32: ERROR:  QueryError: internal error: failed to finish command: channel closed
Traceback (most recent call last):
  File "/home/ubuntu/risingwave/integration_tests/scripts/run_demos.py", line 111, in <module>
    run_demo(args.case, args.format)
  File "/home/ubuntu/risingwave/integration_tests/scripts/run_demos.py", line 42, in run_demo
    run_sql_file(sql_file, demo_dir)
  File "/home/ubuntu/risingwave/integration_tests/scripts/run_demos.py", line 15, in run_sql_file
    proc = subprocess.run(["psql", "-h", "localhost", "-p", "4566",
  File "/usr/lib/python3.10/subprocess.py", line 524, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['psql', '-h', 'localhost', '-p', '4566', '-d', 'dev', '-U', 'root', '-f', '/home/ubuntu/risingwave/integration_tests/tidb-cdc-sink/create_mv.sql', '-v', 'ON_ERROR_STOP=1']' returned non-zero exit status 3

CN node panic log:
cn.log

risingwave image tag: latest(nightly-20230422)

[fuyufjh]

2023-04-23T08:45:44.902827Z ERROR risingwave_stream::task::stream_manager: actor exit actor=72 error=Executor error: Sink error: Remote sink error: Expected timestamp in string, got class java.lang.Double
  backtrace of `StreamExecutorError`:
   0: <risingwave_stream::executor::error::StreamExecutorError as core::convert::From<risingwave_stream::executor::error::Inner>>::from
             at ./risingwave/src/stream/src/executor/error.rs:95:10
   1: <T as core::convert::Into<U>>::into
             at ./rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library/core/src/convert/mod.rs:727:9
   2: <risingwave_stream::executor::error::StreamExecutorError as core::convert::From<risingwave_connector::sink::SinkError>>::from
             at ./risingwave/src/stream/src/executor/error.rs:183:29

[tabVersion]

We need a source of truth table for datatype mappings.

The current implementation is based on pg, according to the doc, it is reasonable to map timestamp into i64 for the time in milli-secs.
But mysql and tidb only accept string in YYYY-MM-DD HH:MM:SS[.fraction] format, leading to the error.
https://docs.pingcap.com/tidb/stable/data-type-date-and-time#datetime-type

So do type date and time, pg accepts them in i32/i64 but mysql only accepts string format

[huangjw806]

We need a source of truth table for datatype mappings

truth table for datatype mappings is here：
https:/risingwavelabs/risingwave/blob/main/integration_tests/tidb-cdc-sink

[huangjw806]

integration test CN panic log:

2023-04-24T01:56:55.437051Z ERROR risingwave_stream::task::stream_manager: actor exit actor=39 error=failed to send message to actor 71: Chunk(StreamChunk { cardinality: 1, capacity: 1, .. })
2023-04-24T01:56:55.437058Z ERROR risingwave_stream::task::stream_manager: actor exit actor=33 error=failed to send message to actor 65: Barrier(Barrier { epoch: EpochPair { curr: 4264179494682624, prev: 4264179429081088 }, mutation: None, checkpoint: false, passed_actors: [1, 49, 42, 33] })
2023-04-24T01:56:55.437079Z ERROR risingwave_compute::rpc::service::stream_service: failed to collect barrier: Actor 40 exit unexpectedly: failed to send message to actor 72: Barrier(Barrier { epoch: EpochPair { curr: 4264179494682624, prev: 4264179429081088 }, mutation: None, checkpoint: false, passed_actors: [1, 49, 44, 40] })
  backtrace of `StreamError`:
   0: <risingwave_stream::error::StreamError as core::convert::From<risingwave_stream::error::Inner>>::from
             at ./risingwave/src/stream/src/error.rs:47:10
   1: <T as core::convert::Into<U>>::into
             at ./rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library/core/src/convert/mod.rs:727:9
   2: <risingwave_stream::error::StreamError as core::convert::From<anyhow::Error>>::from
             at ./risingwave/src/stream/src/error.rs:110:28
   3: <T as core::convert::Into<U>>::into
             at ./rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library/core/src/convert/mod.rs:727:9
   4: <risingwave_stream::executor::exchange::output::LocalOutput as risingwave_stream::executor::exchange::output::Output>::send::{{closure}}::{{closure}}
             at ./risingwave/src/stream/src/executor/exchange/output.rs:83:18
   5: core::result::Result<T,E>::map_err
             at ./rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library/core/src/result.rs:844:27
   6: <risingwave_stream::executor::exchange::output::LocalOutput as risingwave_stream::executor::exchange::output::Output>::send::{{closure}}
             at ./risingwave/src/stream/src/executor/exchange/output.rs:73:9

[fuyufjh]

We need a source of truth table for datatype mappings.

The current implementation is based on pg, according to the doc, it is reasonable to map timestamp into i64 for the time in milli-secs. But mysql and tidb only accept string in YYYY-MM-DD HH:MM:SS[.fraction] format, leading to the error. https://docs.pingcap.com/tidb/stable/data-type-date-and-time#datetime-type

So do type date and time, pg accepts them in i32/i64 but mysql only accepts string format

+1 for "source of truth table for datatype mappings". I've just created an issue here risingwavelabs/risingwave-docs#790

For NoSQL or any format without native support for datetime, I think timestamp in integers is good. But for these with native support of datetime, especially SQL database, I think there is no reason to use integer rather than the datetime type.

[StrikeW]

I think #9957 should have fixed the timestamp type issue. PTAL @WillyKidd

[WillyKidd]

Yes I believe it has fixed the issue :)
using the latest docker image the test has passed with no issues

[WillyKidd]

let me add it to the ci worklfow then we should be able to close this issue :)