Bug when installing (Energy) ADE (sequences)

[gioagu]

Hi,

I've been able to reproduce a bug in the installation process of the Energy ADE (but I think it is in the ADE Manager plugin, actually). I'm using the 3DCityDB suite 2024.0.0 and the latest Energy ADE 2.10, and the 3dcitydb 4.4.1.
Besides the standard "citydb", I've created also another citydb schema, "citydb2".
The search path variable is the standard one: search_path = citydb, citydb_pkg, public

Installation:

Case 1:
I want to install the Energy ADE on "citydb2" but not on "citydb". The installation will fail.

Case 2:
I first install the Energy ADE on "citydb", then I can also install it on "citydb2". here it will succeed, but there is a problem.

(I think) The problem is a bug in the way certain objects (sequences) are created and referenced in the SQL scripts. I think they are NOT schema qualified.
The installer looks for objects (e.g. sequences) in "citydb", and not in the actual installation schema ("citydb2"). So, in case 1, it will fail, but not in case 2. Sequences in "citydb" are used also for tables in "citydb2", which is wrong.

A negative side effect is that this also affects the uninstall process. If I try to first uninstall the ADE from "citydb", it will fail, as tables in "citydb2" make use of sequences in "citydb" instead of the ones in "citydb2".

Please get back to me if the description is not clear enough.
Feel free to move this issue to the 3dcitydb or somewhere else, if applicable.

Looking forward to the fixed version! ;-)

Best regards,

G

PS. Small screenshot as proof. Here "citydb2" is actually "alderaan".

After manually issuing

ALTER TABLE alderaan.ng_dailyschedule ALTER COLUMN id SET DEFAULT nextval('alderaan.ng_dailyschedule_seq'::regclass);

then the correct version is:

[clausnagel]

Thanks for reporting, @gioagu. Can you check this, @yaozhihang?

[yaozhihang]

thanks @gioagu, this is a bug in ADE manager plugin. I'll fix it this weekend.

[yaozhihang]

The issue could be resolved with the following changes in the importer/exporter code. It seems that we need to set the search path during the database connection to ensure that the DDL statements are executed against the correct target database schema.

public String getJDBCUrl(String server, int port, String database) {
     return "jdbc:postgresql://" + server + ":" + port + "/" + database + "?defaultRowFetchSize=10000
                 &reWriteBatchedInserts=true
                 &currentSchema=citydb2,citydb,public,citydb_pkg";
}

I also tested another method to set up the search path as shown below, but it didn't work.

statement.execute("SET search_path TO citydb2,citydb,public,citydb_pkg");

[clausnagel]

Hm, adding the following into the constructor of PostgisADEDBSchemaManager in the ADE Manager Plugin seemed to solve the issue for me:

connection.prepareStatement("SET search_path TO " + schema + ", public").execute();

[gioagu]

Hi guys,
thanks for taking care of this bug! 👍

A small idea/suggestion: In the UNINSTALL DDL scripts of an ADE (the one stored in table ADE), would it be possible to add schema qualified names? This way, the SQL statements would work "out of the box", without need to (re)set the search_path every time.

Or, again, something at the beginning of the script (e.g. "SET search_path TO " + schema + ", public") that have the script work in the right schema, and maybe at the end to set the search_path back to the usual value?

Of course, just an idea, I didn't test it so far... ;-)

[clausnagel]

I guess, adding a SET search_path TO my_schema, public to the beginning of the script is possible, yes. But the uninstall scripts only delete the tables and sequences, not the database functions. So, would this really be helpful?

[gioagu]

Indeed, thanks for pointing it out. I agree with adding SET search_path TO my_schema, public at the beginning of the script (But do no forget to set it back to SET search_path TO citydb, citydb_pkg, public at the end!

Besides, I'd have nothing against adding also the delete function statements, such as
DELETE IF EXISTS my_schema.function(...) CASCADE;
although, when everything works fine, you delete them programmatically during unregistration (if I have understood correctly). But if something goes wrong, you have a plan B. Anyway, just a wish... 🧑‍🎄

G

[yaozhihang]

I tested adding SET search_path TO citydb2, public at the beginning of the script. It doesn't work in my tests. So, the solution with connection.prepareStatement("SET search_path TO " + schema + ", public").execute(); looks good to me.

Besides, adding schema-qualified names to the DDL scripts might reuqire some code changes. We could discuss it in a separate ticket.

[yaozhihang]

A fix 4fb7bae is pushed to the release branch of the ADE manager