feat: FIR-33601: use catalogs table instead of databases table if exists #424
Conversation
alexradzin
force-pushed
the
FIR-33601-catalogs
branch
from
bb4616e to
2968174
Compare
| "WHERE engs.engine_name = ?"; | ||
| private static final String DATABASE_QUERY = "SELECT database_name FROM information_schema.databases WHERE database_name=?"; | ||
| private static final String DATABASE_QUERY = "SELECT %1$s_name FROM information_schema.%1$ss WHERE %1$s_name=?"; |
There was a problem hiding this comment.
Should this be called IS_QUERY or something? Since it's not only for querying databases
There was a problem hiding this comment.
I agree that now the name is not good enough, however IMHO IS_QUERY even worse. What about EXISTENCE_QUERY or, probably INVENTORY_QUERY?
But taking in consideration your next comment all this is irrelevant and we just have to use catalogs instead of databases.
There was a problem hiding this comment.
We can use both suggested names. I'm just thinking to make the naming more general
There was a problem hiding this comment.
Renamed to INVENTORY_QUERY.
|
|
||
| public FireboltEngineInformationSchemaService(FireboltConnection fireboltConnection) throws SQLException { | ||
| this.fireboltConnection = fireboltConnection; | ||
| dbTerm = doesRecordExist(format(DATABASE_QUERY, "table"), "catalogs") ? "catalog" : "table"; |
There was a problem hiding this comment.
Should we use databases in case we don't have catalogs? I don't think table makes sense here
There was a problem hiding this comment.
I do not know. I tried to be backwards compatible. Do you think that we should just forget about the databases table for all environments? BTW if this is correct the all already released drivers are broken.
There was a problem hiding this comment.
But we are here checking if information_schema.catalogs table exists. If it does, we continue to use it. If not, we choose to use information_schema.table, which I believe is invalid and it should be information_schema.databases. Am I missing something?
There was a problem hiding this comment.
You are right; I've fixed a bug. Please review again.
alexradzin
force-pushed
the
FIR-33601-catalogs
branch
from
9d431fc to
1783a23
Compare
|
No description provided.