Skip to content

Conversation

@zabetak
Copy link
Member

@zabetak zabetak commented Jan 14, 2026

Why are the changes needed?

Faster test execution and less coupling with filesystem I/O performance

Does this PR introduce any user-facing change?

No

How was this patch tested?

Run all tests matching grep -Rl "qt:database:derby:"

mvn test -Dtest=TestMiniLlapLocalCliDriver -Pitests -pl itests/qtest -Dqfile=explain_jdbc.q,jdbc_handler.q,external_jdbc_auth.q,external_jdbc_table_typeconversion.q,external_jdbc_view.q,external_jdbc_table_partition.q,external_jdbc_table_perf.q,external_jdbc_table.q,authorization_privilege_objects.q,external_jdbc_rowcount.q,external_jdbc_join_mv.q,external_jdbc_table3.q,external_jdbc_table4.q,dataconnector.q,external_jdbc_table2.q,qt_database_derby.q,jdbc_split_filter.q

@abstractdog
Copy link
Contributor

+1 basically, just an idea: does it make sense to still use file-backed ones and not drop them in case "QTEST_LEAVE_FILES" env var is true? (for letting devs post-check derby db in case of debugging)

@zabetak
Copy link
Member Author

zabetak commented Jan 14, 2026

The idea with QTEST_LEAVE_FILES makes sense but it complicates a bit the code for a need that will not appear very frequently and can be easily hacked during debugging.

I guess we can leave without this feature for the time being till (if ever) we bump into problems that are hard to reproduce locally and would require to download a Derby database from a CI run.

@sonarqubecloud
Copy link

@abstractdog
Copy link
Contributor

The idea with QTEST_LEAVE_FILES makes sense but it complicates a bit the code for a need that will not appear very frequently and can be easily hacked during debugging.

I guess we can leave without this feature for the time being till (if ever) we bump into problems that are hard to reproduce locally and would require to download a Derby database from a CI run.

ack, clean code feels better now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants