feat: Add an ability to read metadata for attached databases

[Destrolaric]

Hi SQLite team! Here are my proposed changes to the reading of metadata. To allow to read of metadata from attached databases https://www.sqlite.org/lang_attach.html ~~This functionality will be optional and disabled by default because it adds more layers [schema].[table] for metadata reading.~~ The reason why this functionality will be useful: It isn't easy to work with attached databases using JDBC API because all metadata reading assumes that we are in the main schema. This is fixable by adding schema information to the existing queries.

[gotson]

Hi, i don't think we should add a pragma for that. The JDBC methods accept a schema, and if it is null then it should not be used for filtering.

[Destrolaric]

Hi, pragma is used only for getSchemas to mark if we read them at all. Other parts just check for null.

[gotson]

Hi, pragma is used only for getSchemas to mark if we read them at all. Other parts just check for null.

i don't think the switch is necessary, we can always retrieve them all

[Destrolaric]

@gotson I have removed pragma now the schema read is by default

[gotson]

Code formatting is failing, can you run mvn spotless:apply ?

[Destrolaric]

Hi, I Added 2 test cases for the default DB and for attaching an additional one after getschemas() call. Also fixed formatting issues and now null schema replaced with main

[gotson]

I think the logic should be enhance to retrieve various entities.

The way the schema parameter work in most of the DB Metadata function is as follows:

schemaPattern - a schema name pattern; must match the schema name as it is stored in the database; "" retrieves those without a schema; null means that the schema name should not be used to narrow the search

For all those functions, we probably need to apply that logic:

1. if schemaPattern is "", then we retrieve objects from the main database. That's a subset of 2.
2. if schemaPattern is null, then we should retrieve objects from all schemas. That means we need to loop through all schemas (getSchemas), and run the statements like table_info, table_xinfo and select from sqlite_master for all those schemas, and join the results. The returned TABLE_SCHEM should be the correct one, not the parameter that was passed to the function.
3. if schemaPattern is a String, we should match it against getSchemas. If found, we get items for that schema only. That's a subset of 2.

That means we need to change the logic for all those functions to loop through all known schemas, optionnaly filtered by the schemaPattern parameter, or using main, in order to call the sqlite functions or special tables with the schema, then join everything.

[Destrolaric]

@gotson Ok, this one looks doable and will probably, be ready sometime next week.

[Destrolaric]

@gotson Hi, sorry for waiting; unfortunately, I had no time to finish the pr before. I have a question. JDBC has some functions like getExportedKeys that require using full schema name and returns everything if null. Do we want to read the data from all existing schemas or to use old behaviour?

[Destrolaric]

Added schema(String, String) for pattern matching, which still requires some unit tests and discussion.

[Destrolaric]

        sql.append("    SELECT").append("\n");
        sql.append("      NAME,").append("\n");
        sql.append("      'GLOBAL TEMPORARY' AS TYPE").append("\n");
        sql.append("    FROM").append("\n");
        sql.append("      sqlite_temp_master").append("\n"); 

Also, what should I do with GLOBAL TEMPORARY tables? They kind of have their scheme now. Converting pr to draft until all this is covered with tests.

[Destrolaric]

@gotson, Hi, again, can we discuss acceptance criteria regarding this pr? Currently, it needs to add some unit tests, and we can probably discuss if anything else is required. Also it looks like this one leads to the significant change of default behaviour, so it probably may require more accurate handling.

[gotson]

@gotson, Hi, again, can we discuss acceptance criteria regarding this pr? Currently, it needs to add some unit tests, and we can probably discuss if anything else is required. Also it looks like this one leads to the significant change of default behaviour, so it probably may require more accurate handling.

Hi, unit tests would be needed yes. What kind of significant changes of default behavior would you foresee though ?

[Destrolaric]

@gotson Well, there are some. Most importantly, if the value of the schema pattern is null, it will show all DBs instead of only 'main', that may look like a significant API change.

[gotson]

@gotson Well, there are some. Most importantly, if the value of the schema pattern is null, it will show all DBs instead of only 'main', that may look like a significant API change.

It's fine, it is more correct

[Destrolaric]

@gotson Sorry, could you look at the current state of pr and see if anything else is required?

[gotson]

@gotson Sorry, could you look at the current state of pr and see if anything else is required?

The PR has conflict, you would need to fix those before I can have a look.

[Destrolaric]

@gotson I resolved the conflict with the recent two commits

[gotson]

@gotson I resolved the conflict with the recent two commits

Thank you. I don't have much time these days, but I will look at this when I have time.

[gotson]

I still see conflicts though. Are you sure you pushed your latest changes?

[Destrolaric]

Weird, for me branch have no conflicts

[gotson]

Weird indeed

[gotson]

My bad, it's only the rebase (which was selected by default) that can't be done.

Squash and merge is ok (and it's what we want).

[gotson]

code formatting is failing, can you fix it with mvn spotless:apply ?

[Destrolaric]

@gotson Fixed

[Destrolaric]

@gotson Hi there! I hope you're doing well. I wanted to reach out and kindly ask if you had a chance to review the pull request.

[gotson]

@gotson Hi there! I hope you're doing well. I wanted to reach out and kindly ask if you had a chance to review the pull request.

It's on my list, unfortunately time is scarce these days. Hopefully i have some time this week to look at it!