RETURNING column name is quoted excessively

[Incanus3]

Expected behavior

• when databaseConfig.platformConfig.allQuotedIntentifiers is false, column name shouldn't be quoted, when true, column name should be quoted once

Actual behavior

• when databaseConfig.platformConfig.allQuotedIntentifiers is false, column name is quoted, when true, column name is quoted thrice

Steps to reproduce

Create any entity (didn't test with other entity yet, will try to create minimal example later), use postgres org.postgresql.Driver, turn on SQL logging, call Database.save(entity). This is especially problematic if you set the @Id column name to be upper-case (which we need because of other db backends), because in that case, when identifier quoting is off, it creates the columns in lower-case, but then it quotes the returning column name, so it tries to find the upper-cased name and fails. If you turn on the identifier quoting, it just fails completely, because the name is surrunded by """.

I'm using ebean version 12.16.1. This is the persistence configuration I'm using:

@Configuration
@Import(JacksonAutoConfiguration::class)
class EgjePersistenceConfiguration {
    @Autowired
    lateinit var objMapper: ObjectMapper

    @Bean
    @ConfigurationProperties(prefix = "datasource.egje")
    fun egjeDataSourceConfig() = DataSourceConfig()

    @Bean
    @ConfigurationProperties(prefix = "database.egje")
    fun egjeDatabaseConfig() = DatabaseConfig().apply {
        name = "egje"
        objectMapper = objMapper
        isDefaultServer = false
        dataSourceConfig = egjeDataSourceConfig()
        platformConfig.isAllQuotedIdentifiers = true // or false

        packages = listOf("cz.sentica.qwazar.kb.registries.person.egje")
    }

    @Bean
    fun egjeDatabase(): Database = DatabaseFactory.create(egjeDatabaseConfig())
}

And the configuration:

database:
  egje:
    ddl-run: true
    ddl-generate: true

datasource:
  egje:
    username: ${EGJEDB_USER:qwazar}
    password: ${EGJEDB_PASS:qwazar}
    url: jdbc:postgresql://${EGJEDB_HOST:postgres_egje}/${EGJEDB_NAME:egjedb}
    driver: org.postgresql.Driver

The entity for which it failed:

@Entity
@DbName("egje")
@Table(name = "EA_EMPLOYEE")
class EgjePerson {
    /** Technické ID - primární klíč */
    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "EMPLOYEE_LLR_ID", nullable = false)
    val id: Long = 0L

    /** Osobní číslo  - záznamy se stejným oscis se týkají té samé osoby */
    @Column(name = "OSCIS", nullable = false)
    var oscis: Int = 0

    /** Pořadové číslo úvazku */
    @Column(name = "LLR_SERIAL_NO", nullable = false)
    val serialNumber: Int = 0

    /** Příznak zda jde o hlavní (1) nebo vedlejší (0) úvazek */
    @Column(name = "MAIN_FLG")
    var mainFlag: Int? = null

    /** Uživatelské jméno - username */
    @Column(name = "LOGIN_NAME", length = 80)
    var login: String? = null

    @Column(name = "DOMAIN", length = 80)
    var domain: String? = null

    /** Kategorie úvazku */
    @Column(name = "CATEGORY", length = 10, nullable = false)
    var category: String = ""

    /** Křestní jméno */
    @Column(name = "FIRST_NAME", length = 30, nullable = false)
    var firstName: String = ""

    /** Příjmení */
    @Column(name = "LAST_NAME", length = 40, nullable = false)
    var lastName: String = ""

    /** Datum ukončení vztahu */
    @Column(name = "END_DATE_IN_ORG")
    var endDate: LocalDate? = null

    /** Emailová adresa */
    @Column(name = "EMAIL", length = 250)
    var email: String? = null

    /** Telefonní číslo (na pevnou linku) */
    @Column(name = "PHONE", length = 250)
    var phone: String? = null

    /** Číslo mobilního telefonu */
    @Column(name = "MOBILE", length = 250)
    var mobile: String? = null

    /** Číslo organizační jednotky */
    @Column(name = "ORG_UNIT_ID")
    var orgUnitId: Int? = null

    /** Příznak zda je o úvazek na centrále (1) nebo na pobočce (0) */
    @Column(name = "IS_CKB")
    var isCkbFlag: Int? = null

    /** Firma - viz ciselnik [OrganizationCode] */
    @Column(name = "ORGANIZATION_CODE", length = 4)
    var organizationCode: String? = null

    /** Název pracovní funkce */
    @Column(name = "FUNCTION_NAME", length = 100)
    var functionName: String? = null

    /** Název pracovní funkce v anglickém jazyce */
    @Column(name = "FUNCTION_NAME_EN", length = 100)
    var functionNameEn: String? = null

    /** Rotační skupina pro personal recovery účely - ABCD. */
    @Column(name = "ROTATION_GROUP", length = 250)
    var rotationGroup: String? = null

    /** Cele jmeno */
    val fullName = "$lastName $firstName"

    override fun toString(): String {
        return "<${this::class.simpleName} title=\"$firstName $lastName ($oscis)\" id=$id source=EGJE>"
    }

    override fun equals(other: Any?) = other is EgjePerson && oscis == other.oscis

    override fun hashCode(): Int = oscis.hashCode()
}

and the error output in case of isAllQuotedIdentifiers = true (when set to false, the RETURNING column is only quoted once, but that's not right either because it isn't qouted when running the DDL, so the case differs)

2022-04-13 06:00:31.649 UTC [57] ERROR:  column ""EMPLOYEE_LLR_ID"" does not exist at character 374
2022-04-13 06:00:31.649 UTC [57] HINT:  Perhaps you meant to reference the column "EA_EMPLOYEE.EMPLOYEE_LLR_ID".
2022-04-13 06:00:31.649 UTC [57] STATEMENT:  insert into "EA_EMPLOYEE" ("OSCIS", "LLR_SERIAL_NO", "MAIN_FLG", "LOGIN_NAME", "DOMAIN", "CATEGORY", "FIRST_NAME", "LAST_NAME", "END_DATE_IN_ORG", "EMAIL", "PHON
E", "MOBILE", "ORG_UNIT_ID", "IS_CKB", "ORGANIZATION_CODE", "FUNCTION_NAME", "FUNCTION_NAME_EN", "ROTATION_GROUP", full_name) values ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19)
        RETURNING """EMPLOYEE_LLR_ID"""

There's also one other strange problem you can notice here - why does it use the full_name as column? It's not annotated with @Column. And it's not quoted in the query even though quoting is on in this case.

[Incanus3]

I just found out, this problem occurs only when using GenerationType.IDENTITY for the id column. When using GenerationType.SEQUENCE, everything works as expected (at least in the unquoted case).

[rbygrave]

Pretty sure this is a postgres jdbc driver bug. Are you using the latest driver? What version?

[rbygrave]

Refer to #2303

[Incanus3]

Wow, thanks for the fast response, you guys are crazy active. I'm using org.postgresql:postgresql -> 42.2.25 which should be the first fixed one, according to the linked issue, but I'll try to upgrade to 42.3 ASAP.

[Incanus3]

I can't seem to be able to find the quoteReturningIdentifiers property though, neither on DatabaseConfig, nor on DataSourceConfig and I'm not creating the connections manually.

[rbygrave]

quoteReturningIdentifiers is a property on the Postgres JDBC Driver - this is not an Ebean property, so yes it does not exist on DatabaseConfig etc. That is, this is an issue with the Postgres JDBC driver in how it is dealing with quoted identifiers with the use of GetGeneratedKeys and their "workaround" / "fix" is to specify that property on the jdbc connection url.

IMO their fix isn't ideal and it would instead be better if they didn't effectively end up double quoting the returning clause (because that is never going to be valid) ... but that is the fix they went with at this stage.

[rbygrave]

pgjdbc references:
pgjdbc/pgjdbc#2223
pgjdbc/pgjdbc#2224

The discussion in 2224 is probably the best background on this issue. Ultimately Postgres supports (crazy/unfortuately) things like: CREATE TABLE t("""crazy""" int)

CREATE TABLE t("""crazy""" int) (i.e. a column named "crazy"), it's still valid in Postgres because the quotes can be escaped. ...

Ultimately this ended up as the pgjdbc quoteReturningIdentifiers property that needs to be explicitly set to false (and Ebean isn't automatically setting that as a custom connection property and we need to set it ourselves explicitly).

[Incanus3]

Ok, thanks for the detailed info. So if I understand this correctly, with the current state of afairs, if I want to use their "fix", I should use the DataSourceConfig.addProperty method to set this option?

[jnehlmeier]

@Incanus3 Yes, if you do not want RETURNING clause to be quoted, you have to set the datasource property quoteReturningIdentifiers to false using addProperty because the default value in pgjdbc is true.

[Incanus3]

Can confirm that after upgrading postgresql driver to v42.3 and adding dataSourceConfig.addProperty("quoteReturningIdentifiers", false) the problem indeed goes away. Thank you so much for the help. And great job in general guys, I really love this ORM. Was using JPA before and never want to go back.

[rbygrave]

Hi @jnehlmeier @Incanus3 - can you have a look at PR - #2681 ... I think we should merge that with the thinking that we effectively always want that property set with Postgres + allQuotedIdentifiers

[Incanus3]

Nice. But I'd almost think you want this always on with postgre - if identifier quoting is off, why would you want to quote the returning ones?

…

On Tue, May 3, 2022, 3:39 AM Rob Bygrave ***@***.***> wrote: Closed #2647 <#2647>. — Reply to this email directly, view it on GitHub <#2647 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUGJOO74X34VF4XT6J2KLVIB7TLANCNFSM5TJSUINA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jnehlmeier]

Hi @jnehlmeier @Incanus3 - can you have a look at PR - #2681 ... I think we should merge that with the thinking that we effectively always want that property set with Postgres + allQuotedIdentifiers

Yes, when ebean quotes everything, then the property must be false. Otherwise you can not save anything through ebean. So the PR is fine.

[rbygrave]

if identifier quoting is off, why would you want to quote the returning ones?

It's more that the Postgres JDBC driver wants to quote returning ones. As I see it, it is safer for apps not using quoted identifiers to leave them as they were. That is, I didn't really want to blanket add the config as that would be a change to applications that haven't needed that configuration property and could potentially break some apps.

[Incanus3]

Ok, that makes sense. And the completely broken case with triple quoting is fixed, so most people shouldn't get tripped up.

…

On Tue, May 3, 2022, 12:02 PM Rob Bygrave ***@***.***> wrote: if identifier quoting is off, why would you want to quote the returning ones? It's more that the Postgres JDBC driver wants to quote returning ones. As I see it, it is safer for apps not using quoted identifiers to leave them as they were. That is, I didn't really want to blanket add the config as that would be a change to applications that haven't needed that configuration property and could potentially break some apps. — Reply to this email directly, view it on GitHub <#2647 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUGJMW7WISQOW4S7MFXQDVID2SVANCNFSM5TJSUINA> . You are receiving this because you were mentioned.Message ID: ***@***.***>