Update oracle test container version to 18.4.0-full

[chenjian2664]

Description

Fix #5901

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.

[ebyhr]

Could you run stress tests? Those scripts were introduced to avoid flaky tests.

[chenjian2664]

Could you run stress tests? Those scripts were introduced to avoid flaky tests.

Thank you! I do not have full context about this scripts, so wondering the reason add the init.sql, it would be appreciated if can share some issues relate to this.
The main thing I want to do this to allow build longer objects names for Oracle tests.(Modifying Oracle part and do not want to do extra works for just modifying the table name to shorter...)
In what condition we can remove the scripts?(like run 10 times, failure 1 or more details requirements). Also, Keep the scripts is totally ok for me.

[ebyhr]

I don't remember the exact number. You can check by removing those scripts without the image bump and run stress tests. I guess it requires at least 30 invocations.

[chenjian2664]

Only update oracle container version at current

[ebyhr]

Fix #5901

There're many TODO comments about this issue. We shouldn't close it in this PR without fixing them.

[hashhar]

If we want we should add new test with newer version since 11.2 is still the oldest supported version by Oracle and we want to make sure that we can find and fix bugs against it.

So I don't think this PR would allow to remove the TODOs anyway since 11.2 still needs to be tested. Sometime after 11.2 is out of support (Dec 2023) we can revisit this PR. For now I think we shouldn't merge this.

[chenjian2664]

@hashhar Make sense. How about:

1. How about fix the TODO in the base test cases and only make the different in the Oracle connector test class
2. For the new version, we could postpone it

[ebyhr]

How about fix the TODO in the base test cases and only make the different in the Oracle connector test class

Do you mean renaming tables in BaseConnectorTest and override each test method in BaseOracleConnectorTest? I disagree if so. It's hard to keep in sync. We may forget to update the Oracle test when changing the base test.

[chenjian2664]

How about fix the TODO in the base test cases and only make the different in the Oracle connector test class

Do you mean renaming tables in BaseConnectorTest and override each test method in BaseOracleConnectorTest?

Yes.

I disagree if so. It's hard to keep in sync. We may forget to update the Oracle test when changing the base test.

We could only override the naming not all the test

[ebyhr]

We could only override the naming not all the test

That's possible, but overriding the naming only for the purpose doesn't make sense. It reduces readability and it doesn't bring any benefit. I would recommend closing this PR now.

[chenjian2664]

We could only override the naming not all the test

That's possible, but overriding the naming only for the purpose doesn't make sense. It reduces readability and it doesn't bring any benefit. I would recommend closing this PR now.

Can. I meet this when want to open the merge feature for the Oracle, there are exist tests cases can not fit Oracle naming constraints, for these cases we shouldn't shorter existing name I think, what do you think?

[ebyhr]

for these cases we shouldn't shorter existing name

Why? Renaming the table names in test shouldn't reduce the readability so much. We can improve variable names or add code comments instead of using longer table names.

[chenjian2664]

for these cases we shouldn't shorter existing name

Why?

It's reducing the readability for the existing test, and the adjust only for the Oracle.

[mahic]

@hashhar Would it make sense to revisit this as 11 is out of support now. Trino is thus running tests on an unsupported version of Oracle. It would be a nice side-effect if merging in a newer version of the test container would result in supporting the latest JDBC driver version

[hashhar]

@mahic Thanks for revisiting this. Yes, I think this makes sense to revive now. Do you already plan on sending a PR?

[chenjian2664]

@hashhar @ebyhr Do you think it's worth to reopen it? I am still facing the naming issues while modifying the Oracle tests