The implementations of interface XTablesSupplier.getTables() in LibreOffice currently return a XNameAccess that uses: schemaName + "." + tableName as "name" to access the tables. However, this scheme can make collisions between schemaName == "foo" tableName == "bar.qux" and schemaName == "foo.bar" tableName == "qux" because in both cases, the name resolves to "foo.bar.qux". The XNameAccess should rather use properly quoted and escaped names, that is: '"' + escapeDoubleQuotes(schemaName) + '"."' + escapeDoubleQuotes(tableName) + '"' double quotes should be escaped by repeating them. At the same time, all callers to this interface have to be changed, too. The whole name construction business should be done by a utility function, most probably ::dbtools::composeTableNameForSelect (which needs to be changed to handle double quotes in names...). Implementations of XTablesSupplier: 1) MySQL driver: I haven't found where the name is constructed; possibly in db-agnostic dbaccess/ or connectivity/source/sdbcx or connectivity/source/commontools code. 2) PostgreSQL driver: pq_xtables.cxx function Tables::refresh code snippet: buf.append( schema ).appendAscii( "." ).append( name ); map[ buf.makeStringAndClear() ] = currentTableIndex; callers that need to be also modified: pq_resultsetmetadata.cxx function ResultSetMetaData::checkTable
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility. see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Adding self to CC if not already on
Migrating Whiteboard tags to Keywords: (easyHack, difficultyInteresting, skillCpp, skillSql) [NinjaEdit]
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
I find this task interesting, I want to solve it as my first easy hack. Can anyone please give me some pointer to file to start working on it so that I can assign this task to myself.
In connectivity/source/drivers/*, each directory should contain one implementation of getTables() from the XTablesSupplier interface. This does not concern the getTables from the XDatabaseMetaData interface. See http://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1sdbc_1_1XDatabaseMetaData.html#aee9c7e3149e149b3694c900bd6a0671f http://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1sdbcx_1_1XTablesSupplier.html#a364c9ed144725a9c2fdb159e2c201e19 The easy way to make the difference between the two is that the one we care about in this bug does not take any arguments, while the other one does. So we need to look for (e.g. with "git grep") getTables definitions and invocations without any argument. Each needs to be changed. Then everywhere getTables (without arguments) is *called* needs to be changed. For the first part, the likely matches are: connectivity/source/drivers/evoab2/NCatalog.cxx connectivity/source/drivers/kab/KCatalog.cxx connectivity/source/drivers/macab/MacabCatalog.cxx connectivity/source/drivers/mork/MCatalog.cxx connectivity/source/drivers/postgresql/pq_connection.cxx connectivity/source/sdbcx/VCatalog.cxx dbaccess/source/core/api/SingleSelectQueryComposer.cxx dbaccess/source/core/api/querycomposer.cxx dbaccess/source/core/dataaccess/connection.cxx dbaccess/source/core/dataaccess/datasource.cxx When these return a pre-made object (which I expect to be common), you need to go look where that object is created. For example, according to comment 0, for postgresql/pq_connection.cxx, this is in pq_xtables.cxx function Tables::refresh I suggest you take them one by one, and we can discuss them one by one.
Assigning to myself
Created attachment 124855 [details] Attaching changes made in some file for escaped schema name and table name. Tell me I am going in right direction. I have attached patch file for postgre driver. I am not sure if I am going in right direction that's why uploading patch here. Currently not followed all coding conventions. Please tell me changes so that I can make it better.
(In reply to Prashant from comment #8) > Created attachment 124855 [details] > Attaching changes made in some file for escaped schema name and table name. > Tell me I am going in right direction. > > I have attached patch file for postgre driver. I am not sure if I am going > in right direction that's why uploading patch here. Currently not followed > all coding conventions. Please tell me changes so that I can make it better. Please submit the patch to gerrit, that is where we do reviews. have a look at: https://wiki.documentfoundation.org/Development/GetInvolved and ping me if you need help.
(In reply to jan iversen from comment #9) > Please submit the patch to gerrit, that is where we do reviews. When you do that, please: 1) clearly mark it as WiP (Work in Progress), as long as you don't think it ready for merging, only looking for feedback. 2) add me as reviewer (In reply to Prashant from comment #8) > Created attachment 124855 [details] > Attaching changes made in some file for escaped schema name and table name. > Tell me I am going in right direction. From a first glance, yes it looks like going in the right direction. However, instead of quoting by calling .replaceAll("\"","\"\"" ) I'd suggest using ::dbtools::composeTableNameForSelect (see include/connectivity/dbtools.hxx) For the case of PostgreSQL, since it does not support catalogs, just pass an empty catalog. In pq_resultsetmetadata.cxx: good, this is indeed a name access on an object returned by getTables() The changes in pq_statement.cxx do not look to me like they belong to this bug, nor are they consistent: 1) In function executePostgresCommand, you change the treatment of sourceTable, but sourceTable comes from extractSingleTableFromSelect, and I don't see you adding any escaping there. Actually, reading the code, extractSingleTableFromSelect removes the enclosing '"' without undoubling the inner '"', so it looks like another related bug there. If you want, you can fix that other bug in a separate patch (upload it to gerrit and put me as reviewer). 2) In function getGeneratedValuesFromLastInsert, you change the treatment of the argument lastTableInserted, but I don't see you changing the value passed. Unless I have missed something, this is inconsistent pq_tools.cxx function splitDoubleQuoteEscapedIdentifiers: why do you go through an OString instead of doing everything on OUStrings directly?
A polite ping, still working on this issue ?
Prashant committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=d43f4390e006716ca538ad92d09bd012179efd66 tdf#67302 Resolving tablesSupplier name clash for postgresql It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Seems solved
Lionel Elie Mamane committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=b6976604ca15259af3a3ee95e10d24937bd63b9a Revert "tdf#67302 Resolving tablesSupplier name clash for postgresql" It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Following the revert (see https://cgit.freedesktop.org/libreoffice/core/commit/?id=b6976604ca15259af3a3ee95e10d24937bd63b9a), let's reopen this one.
Can you please provide an example of how to exhibit this bug by using the interface, and how to see the output?
(In reply to Matt K from comment #16) > Can you please provide an example of how to exhibit this bug by using the > interface, and how to see the output? I'm not sure it can be exposed in the UI per se. But if one creates a schema with a "." in the name, and a table with a "." in the name, to trigger the name clash, possibly/probably bugs will start to appear in the UI, in the form of one being used when one means the other. To trigger the name clash, e.g.: - create two schemas "a" and "a.b" - in schema "a", create table "b.c" - in schema "a.b", create table "c" Make the two tables different, at least different data if not different structure. Then try to open one, the other, LibreOffice may be confused between the two, not sure.
Can you explain how to create the schema, is it done through an outside program? I created 2 tables in LO with the same table name/column name pattern and the data stays separate between the tables.
(In reply to Matt K from comment #18) > Can you explain how to create the schema, is it done through an outside > program? Database systems will differ a bit on that, but usually the SQL command is something like: CREATE SCHEMA "name"; > I created 2 tables in LO with the same table name/column name > pattern and the data stays separate between the tables. Exactly same table name is not supposed to be possible, except in different schema or catalog?