Description: In Base if you create a new table with: Insert -> Table Design... When saving this new table with: File -> Save As If you modify the schema name in the drop-down list, the default table name will not be updated and will not take into account the tables contained in this new selected schema. This results in a possible conflict on the name of the tables... Steps to Reproduce: 1. In Base create a new table with: Insert -> Table Design... 2. Save this new table with: File -> Save As 3. In this Save As dialog change the schema name. Actual Results: When you change the schema name in the Save As dialog box, the default table name will not be recalculated. Expected Results: When you change the schema name in the Save As dialog box, the default table name must be recalculated. Reproducible: Always User Profile Reset: No Additional Info: Normally UNO offers a method in the SDBC API to find the next available table name under a given schema.
I don't have a database driver capable of managing several catalogs, but I believe that the same problem exists when changing the catalog. This remains to be confirmed...
There is no schema name available for internal databases. So I tested it with PostgreSQL and direct connection. Created a new table. Changed schema from "public" to another schema I have writing access. Table will appear directly after saving the table. Tested with Version: 7.4.7.2 / LibreOffice Community Build ID: 723314e595e8007d3cf785c16538505a1c878ca5 CPU threads: 6; OS: Linux 5.14; UI render: default; VCL: kf5 (cairo+xcb) Locale: de-DE (de_DE.UTF-8); UI: de-DE Calc: threaded
Hi Robert, If we use the default names proposed by Base (Table1, Table2, Table...) the conflict only occurs when we create the second table in this same schema and this schema is not selected by default in the save as dialog box...
(In reply to Robert Großkopf from comment #2) > There is no schema name available for internal databases. Yes, I forgot to specify that I have this problem only with my JDBC driver and under all the databases that I could integrate, namely: - hsqldb - h2 - mariadb - sqlite I even think it's the same in OpenOffice...
I would like to find the code for the Save As dialog? Because I think there is no handler on the drop-down list of schemas allowing the update of the default table name...
(In reply to prrvchr from comment #5) > I would like to find the code for the Save As dialog? > > Because I think there is no handler on the drop-down list of schemas > allowing the update of the default table name... I didn't check but perhaps https://opengrok.libreoffice.org/search?project=core&full=&defs=OSaveAsDlg&refs=&path=&hist=&type=&xrd=&nn=1&si=defs&si=defs may help. You can put breakpoint + use "bt" in gdb and/or add some fprintf.
(In reply to prrvchr from comment #3) > Hi Robert, > > If we use the default names proposed by Base (Table1, Table2, Table...) the > conflict only occurs when we create the second table in this same schema and > this schema is not selected by default in the save as dialog box... Hi prrvchr, have tested with PostgreSQL direct driver, PostgreSQL JDBC connection, MariaDB JDBC connection. Could create as many tables as I want in the schema, which isn't selected as default schema. All tables will directly appear in the schema.
(In reply to Julien Nabet from comment #6) > https://opengrok.libreoffice.org/ > search?project=core&full=&defs=OSaveAsDlg&refs=&path=&hist=&type=&xrd=&nn=1&s > i=defs&si=defs may help. If it is the dialog that saves the table name, and I believe it is, the default table name seems to be loaded when the dialog is initialized (const OUString& rDefault) with a unique full table name (catalog.schema.tablename) and nothing seems to be done after changing the catalog name or the schema name. Now I could be wrong, I don't know much about C. (In reply to Robert Großkopf from comment #7) > have tested with PostgreSQL direct driver, PostgreSQL JDBC connection, > MariaDB JDBC connection. Could create as many tables as I want in the > schema, which isn't selected as default schema. All tables will directly > appear in the schema. If I use MariaDB JDBC (mariadb-java-client-3.1.4.jar with LibreOffice JDBC native driver) and I let Base choose the name of the table (table1 by default), it only works the first time (when table1 does not exist) but fails the second time (see the screenshot of the error) The only driver for which I have no problem is the SQLite JDBC driver (sqlite-jdbc-3.42.0.0.jar) under jdbcDriverOOo (the automatic numbering of table names works well) and this is due to the fact that SQLite does not support either schema or catalog and therefore the uniqueness of the name of the tables is therefore ensured.
Created attachment 188988 [details] Base Save AS screenshot error
Reading the file again: dlgsave.cxx, it seems that m_xMetaData->getUserName() is an Oracle JDBC specification. There is no reason for the connection user's name to be called here, except to use an Oracle JDBC driver. see: https://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/dlgsave.cxx?r=75fcdffd#137 The UNO API does not seem to be able to retrieve the default schema of the connection, however there is the getSchema() method in JDBC since Java 7 see: https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#getSchema--
Ah, now I see the problem you got. I understood: Table won't appear. But you wanted to save the name automatically without changing the entry. Only in internal databases without any schema the file name of the table will be changed automatically. I get the same behavior without changing schema: Table name will always be "Table1". If I try to save a new table in the same schema it will be the same name - also in default schema.
(In reply to Robert Großkopf from comment #11) > Ah, now I see the problem you got. I understood: Table won't appear. But you > wanted to save the name automatically without changing the entry. > > Only in internal databases without any schema the file name of the table > will be changed automatically. > > I get the same behavior without changing schema: Table name will always be > "Table1". If I try to save a new table in the same schema it will be the > same name - also in default schema. This bug causes two faults to appear: - Base is not able to know the default schema of the connection, if the connection supports schemas. - dlgsave.cxx does not ensure that the table name remains unique when changing schema or catalog in combobox. To solve the first defect, it would be necessary to add a new method getSchema() to the idl file com.sun.star.sdbc.XConnection (same as getCatalog()) The second problem should be solved by listeners on the combobox allowing to recalculate the unique name of the table according to the changed schema and / or catalog. Maybe there is another easier way to fix this bug?
> To solve the first defect, it would be necessary to add a new method getSchema() > to the idl file com.sun.star.sdbc.XConnection (same as getCatalog()) If we are forced to come to this, then we must also consider adding the method: - setSchema(String) to the idl file com.sun.star.sdbc.XConnection
Lionel: thought you might be interested in the suggestions from the last 2 comments.
(In reply to prrvchr from comment #10) > The UNO API does not seem to be able to retrieve the default schema of the > connection, however there is the getSchema() method in JDBC since Java 7 > see: > https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#getSchema- > - SDBC is basically a C++/UNO "version/copy/transcription" of some older JDBC version. Not staying frozen back in time and resynchronising with newer JDBC versions would be a welcome project. Due to our ABI compatibility rules, this may (stronger, I expect it will) require the use of versioned API that extend the existing ones, see e.g. css::sdbc::XDatabaseMetaData2 which extends css::sdbc::XDatabaseMetaData css::sdbc::XDriverManager2 which extends css::sdbc::XDriverManager for the mechanism which I mean. To avoid having a multitude of css::sdbc::Foo css::sdbc::Foo1 css::sdbc::Foo2 css::sdbc::Foo3 css::sdbc::Foo4 css::sdbc::Foo5 css::sdbc::Foo6 I think it would be nice to do the "resynchronisation" in one go rather than adding the new member functions one by one as we keep "discovering" the need for them.
(In reply to Lionel Elie Mamane from comment #15) > I think it would be nice to do the "resynchronisation" in one go rather than > adding the new member functions one by one as we keep "discovering" the need > for them. I am ready to make all the necessary changes. If I understood you correctly, this would be done through a new interface com.sun.star.sdbc.XConnection2 inheriting from the interface com.sun.star.sdbc.XConnection and having the following methods: - string getSchema() - void setSchema(string Schema) Maybe we should also consider: - Any setSavepoint(string Name) - void releaseSavepoint(Any Savepoint) - void rollbackToSavepoint(Any Savepoint) And I also thought of adding access to the underlying database logging with: - com.sun.star.logger.XLogger getLogger() Or maybe this should be added to the com.sun.star.sdbc/x.XDriver as it is done with Java ? Surely I forgot things?
(In reply to prrvchr from comment #16) > (In reply to Lionel Elie Mamane from comment #15) >> I think it would be nice to do the "resynchronisation" in one go rather than >> adding the new member functions one by one as we keep "discovering" the need >> for them. > I am ready to make all the necessary changes. > If I understood you correctly, this would be done through a new interface > com.sun.star.sdbc.XConnection2 inheriting from the interface > com.sun.star.sdbc.XConnection and having the following methods: > - string getSchema() > - void setSchema(string Schema) If these are the only missing in LibreOffice's css::sdbc::XConnection when compared to Java's java.sql.Connection, then yes. Else, please add in one go all missing ones in the new css::sdbc::XConnection2 Thanks! > Maybe we should also consider: > - Any setSavepoint(string Name) > - void releaseSavepoint(Any Savepoint) > - void rollbackToSavepoint(Any Savepoint) Well, you give examples yourself of other things missing, so please add them all. My point was to _not_ only add "get/setSchema" because we need them now, and then in 3 months or 3 years, we want set/release/rollbackToSavepoint and we create css::sdbc::XConnection3 for those. I would prefer we create _now_ css::sdbc::XConnection2 with get/setSchema, set/release/rollbackToSavepoint and anything else missing in LibreOffice's SDBC(X) when compared to the last version of JDBC (according to Wikipedia that's JDBC 4.3 present since Java 9, and wouldn't have changed since then in newer versions of Java). > > And I also thought of adding access to the underlying database logging with: > - com.sun.star.logger.XLogger getLogger() > Or maybe this should be added to the com.sun.star.sdbc/x.XDriver as it is > done with Java ? > > Surely I forgot things?
(In reply to prrvchr from comment #16) > And I also thought of adding access to the underlying database logging with: > - com.sun.star.logger.XLogger getLogger() > Or maybe this should be added to the com.sun.star.sdbc/x.XDriver as it is > done with Java ? Sorry, forgot to answer this one. I don't have out of the top of my head enough knowledge to give a firm opinion immediately. Do what you feel would be most clean, not only in terms of wrapping JDBC Java drivers, but also pure SDBC/C++ drivers, the ODBC wrapper, the ADO wrapper, etc.
(In reply to Lionel Elie Mamane from comment #18) > > I don't have out of the top of my head enough knowledge to give a firm > opinion immediately. Do what you feel would be most clean, not only in terms > of wrapping JDBC Java drivers, but also pure SDBC/C++ drivers, the ODBC > wrapper, the ADO wrapper, etc. I think it's a shame to have to establish a connection in order to have access to the logging of the underlying driver. Perhaps it would be more appropriate for the getLogger() method to be attached to the UNO com.sun.star.sdbc.XDriver and to add a URL as a parameter to be able to find the underlying logger. (ie: jdbc:hsqldb)
Maybe there is an appropriate method to start fixing this bug without interfering with my previous fixes?
(In reply to prrvchr from comment #20) > Maybe there is an appropriate method to start fixing this bug without > interfering with my previous fixes? You can work separately on different tasks locally by creating local branches with git. Of course, it requires a bit more of management, eg: to be sure to be sync with master repo.
(In reply to Julien Nabet from comment #21) > > You can work separately on different tasks locally by creating local > branches with git. > Of course, it requires a bit more of management, eg: to be sure to be sync > with master repo. Thanks for all the help, I couldn't have done anything without it. I have created a new file: com.sun.star.sdbc.XConnection2.idl, but I have an error during the compilation: the new methods are not found. Maybe it is necessary to declare this new idl file somewhere?
(In reply to prrvchr from comment #22) > I have created a new file: com.sun.star.sdbc.XConnection2.idl. > Maybe it is necessary to declare this new idl file somewhere? offapi/UnoApi_offapi.mk
Although I added XConnection2 to the file offapi/UnoApi_offapi.mk, I got an error: /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx: In constructor ‘dbaui::OSaveAsDlg::OSaveAsDlg(weld::Window*, sal_Int32, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>&, const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>&, const rtl::OUString&, const dbaui::IObjectNameCheck&, SADFlags)’: /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx:134:41: error: ‘class com::sun::star::sdbc::XConnection’ has no member named ‘getSchema’ 134 | sSchema = _xConnection->getSchema(); | ^~~~~~~~~ I missed something...
For discussion on code, you can use gerrit's "draft" or "work in progress" feature (which makes the change not visible publicly, and I think also not auto-built by Jenkins), and then invite the persons you want to discuss with. See https://gerritcodereview-test.gsrc.io/marking-a-change-as-a-wip.html (In reply to prrvchr from comment #24) > Although I added XConnection2 to the file offapi/UnoApi_offapi.mk, I got an > error: > /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx: In > constructor ‘dbaui::OSaveAsDlg::OSaveAsDlg(weld::Window*, sal_Int32, const > com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>&, > const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>&, > const rtl::OUString&, const dbaui::IObjectNameCheck&, SADFlags)’: > /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx:134:41: > error: ‘class com::sun::star::sdbc::XConnection’ has no member named > ‘getSchema’ > 134 | sSchema = _xConnection->getSchema(); > | ^~~~~~~~~ > > I missed something... _xConnection is a reference to com::sun::star::sdbc::XConnection while getSchema() is defined in com::sun::star::sdbc::XConnection2. You need to cast _xConnection to a (reference to a) com::sun::star::sdbc::XConnection2 _and_ since there is not guarantee that the object referred to by _xConnection implements com::sun::star::sdbc::XConnection2, before using it, check that the cast gave a non-NULL result. 1. Add #include <com/sun/star/sdbc/XConnection2.hpp> in the list of #includes at the beginning of the file 2. The cast actually happens rather automatically, IIRC the code to write is: const Reference< XConnection2 > xCon2(_xConnection); if (xCon2.is()) { sSchema = _xConnection->getSchema(); } else { deal with the situation otherwise... e.g. as it is done now }
I managed to put the correction in work in progress: https://gerrit.libreoffice.org/c/core/+/155950 But I still get an error while compiling: /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx: In constructor ‘dbaui::OSaveAsDlg::OSaveAsDlg(weld::Window*, sal_Int32, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>&, const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>&, const rtl::OUString&, const dbaui::IObjectNameCheck&, SADFlags)’: /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx:136:56: error: invalid initialization of reference of type ‘const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection2>&’ from expression of type ‘const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>’ 136 | const Reference< XConnection2 >& xCon2(_xConnection); | ^~~~~~~~~~~~ I'm sorry, I'm going to have to learn, thanks for your help...
(In reply to prrvchr from comment #26) > I managed to put the correction in work in progress: > > https://gerrit.libreoffice.org/c/core/+/155950 > > But I still get an error while compiling: > > /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx: In > constructor ‘dbaui::OSaveAsDlg::OSaveAsDlg(weld::Window*, sal_Int32, const > com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>&, > const com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>&, > const rtl::OUString&, const dbaui::IObjectNameCheck&, SADFlags)’: > /home/prrvchr/github/libreoffice/dbaccess/source/ui/dlg/dlgsave.cxx:136:56: > error: invalid initialization of reference of type ‘const > com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection2>&’ from > expression of type ‘const > com::sun::star::uno::Reference<com::sun::star::sdbc::XConnection>’ > 136 | const Reference< XConnection2 >& xCon2(_xConnection); > | ^~~~~~~~~~~~ > > I'm sorry, I'm going to have to learn, thanks for your help... I can't succeed in commenting the gerrit patch, wonder if it's because it's WIP. I don't want to click on Start Review because I think it'll remove WIP status. Anyway, changing: const Reference< XConnection2 > xCon2(_xConnection); by: const Reference< XConnection2 > xCon2(_xConnection, UNO_QUERY); or const Reference< XConnection2 > xCon2(_xConnection, UNO_QUERY_THROW); should work. There are some explanation here: https://opengrok.libreoffice.org/xref/core/include/com/sun/star/uno/Reference.h?r=53c39db4&mo=4987&fi=157#157 now I'm not sure which one we should use and why.
(In reply to Julien Nabet from comment #27) > I don't want to click on Start Review because I think it'll remove WIP > status. Click there, then "Send as WiP" (not "Send and Start Review", that's what I did). > const Reference< XConnection2 > xCon2(_xConnection, UNO_QUERY); > or > const Reference< XConnection2 > xCon2(_xConnection, UNO_QUERY_THROW); > now I'm not sure which one we should use and why. That's important to understand. UNO_QUERY (which I think/hope is the default and can just be omitted, but I'm a bit rusty on details) means that if _xConnection does not implement the XConnection2 interface, then xCon2 will be a NULL reference. UNO_QUERY_THROW means that if _xConnection does not implement the XConnection2 interface, then a RuntimeException will be thrown; execution of the code block is _stopped_, and everything goes up scope by scope until the exception is caught. Generally, UNO_QUERY is good when the situation (the requested interface is not implemented) is handled locally, with an "if (xCon2.is())" as I suggested should be done in this case. Generally, UNO_QUERY_THROW is good when the code just cannot continue in this situation, and the error must go up the stack until the error is handled.
Thanks for your help. This first part is done on the other hand it remains: - Setting up a listener on the two comboboxes? - The generation of the default name of the table which must take into account the default catalog as well as the default schema. I have no idea about the handler/listener implementation on comboboxes. It seems to me that the default table name should be constructed there: https://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/tabledesign/TableController.cxx?r=91ba9654#275 but I don't really understand the code...
The integration in Base is finished. There remains, despite everything, the management of the combobox. I'm stuck for adding XConnection2 interface in the file: connectivity/source/drivers/jdbc/JConnection.cxx
I can't help about the comboboxes but you can use https://wiki.documentfoundation.org/Development/Mailing_List or IRC (see https://wiki.documentfoundation.org/Development/GetInvolved)
I just saw that the issue 156816 has been accepted. Maybe we can submit this one? If yes, how to do it? Thanks.
(In reply to prrvchr from comment #32) > I just saw that the issue 156816 has been accepted. You meant the patch concerning tdf#156816 (https://gerrit.libreoffice.org/c/core/+/155866) has been accepted. > Maybe we can submit this one? "This one", you mean patch https://gerrit.libreoffice.org/c/core/+/155950 > > If yes, how to do it? > Thanks. The patch is tagged "WIP" (Work In Progress) and Lionel commented the patch https://gerrit.libreoffice.org/c/core/+/155950 Could you read his comments and apply or respond?
Sorry Julien, I went too quickly I hadn't seen Lionel's comments. I just responded to them. However, I haven't found a way to respond to comments without resubmitting the fix?
(In reply to prrvchr from comment #34) > Sorry Julien, I went too quickly I hadn't seen Lionel's comments. > I just responded to them. > > However, I haven't found a way to respond to comments without resubmitting > the fix? If you change something, you amend the patch (see "amend" option), so you must resubmit then. If you don't change the code and just respond to the comments, these are saved as draft and then when clicking "Reply" it publishes your responses, no resubmit here.
(In reply to prrvchr from comment #34) > However, I haven't found a way to respond to comments without resubmitting > the fix? For a WiP change (patch), hit "reply" and click the "save as WiP" button.
Well I have to admit I'm stuck on adding an implemented UNO interface... https://gerrit.libreoffice.org/c/core/+/155950/7/connectivity/source/inc/TConnection2.hxx
Sorry for the absence, but I'm ready to submit the patch: https://gerrit.libreoffice.org/c/core/+/155950
Hi all, What do we do with this? Are we going to give up?
I can't help here=>uncc myself.