feat(CI): add e2e test of extended query mode in CI

[ZENOTME]

trafficstars

Is your feature request related to a problem? Please describe. The extended query mode now already can run most of our original e2e test. We can add it in the ci.

Describe the solution you'd like

1. add e2e_test_extended in e2e_test folder, (e2e_test/e2e_test_extended)

   The e2e_test_extended is copy of e2e_test. The difference is: due to our extended query mode can't support {array, struct, interval, Decimal::Infinity} type now, I ignore some test case in e2e_test_extended by name these test case '.slt.part.un' instead of '.slt.part'. (When we support more type, we will reuse these test case. When e2e_test_extended support all test case, we can directly use e2e_test.)

2. add new CI script in ci/scripts

• e2e-source-test-extended.sh
• e2e-test-parallel-extended.sh
• e2e-test-extended.sh

[skyzh]

I think we can directly replace the old ones. We don’t need to run the same test twice.

[skyzh]

Also I’m thinking of adding coverage for e2e, so that we can know if we can actually improve coverage by having more tests…

[ZENOTME]

I think we can directly replace the old ones. We don’t need to run the same test twice.

Did u means we can replace the test case which extended query mode can run. And remaining the test case which extended query mode can't run. Or we can wait util the extended query support all type and it can run all original e2e test?

[ZENOTME]

Also I’m thinking of adding coverage for e2e, so that we can know if we can actually improve coverage by having more tests…

I will take a look soon. Sounds like another feature?

[BowenXiao1999]

Also I’m thinking of adding coverage for e2e, so that we can know if we can actually improve coverage by having more tests…

I think we have considered this a long time ago. But seems that coverage of e2e is hard to collect, due to some rust problem?

Sounds like another feature?

Yes I think so

[BowenXiao1999]

I think we can directly replace the old ones. We don’t need to run the same test twice.

The problem of current extended mode is it can not replace all e2e tests now. But indeed same test twice looks costful. @ZENOTME do we only lack the type support for array? If this is the case, I think we can add SET query_mode to simple etc to array*.slt to bypass this or we just impl it 😇 .

[ZENOTME]

I think we can directly replace the old ones. We don’t need to run the same test twice.

The problem of current extended mode is it can not replace all e2e tests now. But indeed same test twice looks costful. @ZENOTME do we only lack the type support for array? If this is the case, I think we can add SET query_mode to simple etc to array*.slt to bypass this.

lack:

• array
• interval
• Decimal::Infinity

[skyzh]

due to some rust problem?

It should be easy now. nextest can support this. We just need time to investigate how to achieve.

[skyzh]

The problem of current extended mode is it can not replace all e2e tests now.

Can we set engine in each slt file?

[ZENOTME]

The problem of current extended mode is it can not replace all e2e tests now.

Can we set engine in each slt file?

Yes. But we can't set for each .slt.part. There are lots of '.slt.part.un'

[BowenXiao1999]

The problem of current extended mode is it can not replace all e2e tests now.

Can we set engine in each slt file?

I think currently no but should be easy to support. The only concern is we might have to set engine back and forth due to all tests are running in one session.

[skyzh]

I think it's easy to cover all code path of pgwire if we have a dedicated slt file to test all kinds of SQL statements (e.g. create MV, batch query, insert). Personally I don't want to duplicate all e2e test cases solely to test out the extended query mode.

[ZENOTME]

If this is the case, I think we can add SET query_mode to simple etc to array*.slt to bypass

We have another design to solve this problem is having '`SET query_mode to simple' in the .slt.part which run in simple query mode. Here is my thought: 'SET query_mode to simple' is a 'switch' in sqllogictest. It control sqllogictest run in simple query mode in the '.slt.part'. It provides ‘slt.part’-grained control so that we can run different query mode in different 'slt.part'.

[xxchan]

BTW, I think maybe pgwire can be tested using mock database, instead of relying on risingwave tests? 🤔 It's an independent library and can be published alone.

[BowenXiao1999]

BTW, I think maybe pgwire can be tested using mock database, instead of relying on risingwave tests? 🤔 It's an independent library and can be published alone.

Yes, I agree. But if we want to test it with different Mock database, we still need to write some code to connect pgwire and database. BTW we may also need to write the test for mock database, cuz they do not have test written in extended query mode but only simple query mode.

[ZENOTME]

Yes, I agree. But if we want to test it with different Mock database, we still need to write some code to connect pgwire and database.

Is this Mock database the Mock Session we use in unit test?

[BowenXiao1999]

Yes, I agree. But if we want to test it with different Mock database, we still need to write some code to connect pgwire and database.

Is this Mock database the Mock Session we use in unit test?

That's just a naive unit test version. The more advanced one is running other database (like DuckDB) under our pgwire

[ZENOTME]

I see. Sounds we need to write a connector which forward the request from pgwire to other database. And forward the response from other database to pgwire.

[fuyufjh]

Is it done? @BowenXiao1999 @ZENOTME

[BowenXiao1999]

Is it done? @BowenXiao1999 @ZENOTME

I think so, we are now running a extended mode test on ./e2e_test/extended_query/basic.slt. But there are some remaining works (some type serialization/deserialization) to fully use extended mode (replace previous simpe query mode) in CI. @ZENOTME Can you show what's the missing part?

[ZENOTME]

The missing part track in #3676 . I think we can close this issue now.