jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.5k stars 4.02k forks source link

Bad SQL grammar is thrown when sorting using foreign key #23847

Closed rjbgaspar closed 10 months ago

rjbgaspar commented 12 months ago

Hi,

Overview of the issue

Unable to sort on foreign key, an exception is thrown because the ORDER BY contains an invalid column.

The generated SQL is as follow:

SELECT
    e.id AS e_id,
    e.name AS e_name,
    e.artist_id AS e_artist_id,
    e.genre_id AS e_genre_id,
    artist.id AS artist_id,
    artist.name AS artist_name,
    genre.id AS genre_id,
    genre.name AS genre_name
FROM
    album e
LEFT OUTER JOIN artist artist ON
    e.artist_id = artist.id
LEFT OUTER JOIN genre genre ON
    e.genre_id = genre.id
ORDER BY
    e_artist.name DESC,
    e_id ASC

As shown Column "E_ARTIST.NAME" is not found, it should be ARTIST.NAME.

Exception detail

2023-10-14T08:18:10.812+01:00 ERROR 85473 --- [     parallel-3] o.z.problem.spring.common.AdviceTraits   : Internal Server Error 

org.springframework.r2dbc.BadSqlGrammarException: executeMany; bad SQL grammar [SELECT e.id AS e_id, e.name AS e_name, e.artist_id AS e_artist_id, e.genre_id AS e_genre_id, artist.id AS artist_id, artist.name AS artist_name, genre.id AS genre_id, genre.name AS genre_name FROM album e LEFT OUTER JOIN artist artist ON e.artist_id = artist.id LEFT OUTER JOIN genre genre ON e.genre_id = genre.id ORDER BY e_artist.name DESC, e_id ASC LIMIT 20 OFFSET 0]; nested exception is io.r2dbc.spi.R2dbcBadGrammarException: [42122] [42S22] Column "E_ARTIST.NAME" not found; SQL statement:
SELECT e.id AS e_id, e.name AS e_name, e.artist_id AS e_artist_id, e.genre_id AS e_genre_id, artist.id AS artist_id, artist.name AS artist_name, genre.id AS genre_id, genre.name AS genre_name FROM album e LEFT OUTER JOIN artist artist ON e.artist_id = artist.id LEFT OUTER JOIN genre genre ON e.genre_id = genre.id ORDER BY e_artist.name DESC, e_id ASC LIMIT 20 OFFSET 0 [42122-214]
    at org.springframework.r2dbc.connection.ConnectionFactoryUtils.convertR2dbcException(ConnectionFactoryUtils.java:235)
    Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Assembly trace from producer [reactor.core.publisher.MonoError] :

Error has been observed at the following site(s):
    *_____Flux.onErrorMap ⇢ at org.springframework.r2dbc.core.DefaultDatabaseClient.inConnectionMany(DefaultDatabaseClient.java:146)
    *______Flux.usingWhen ⇢ at org.springframework.transaction.interceptor.TransactionAspectSupport$ReactiveTransactionSupport.lambda$null$8
Motivation for or Use Case

n.a.

Reproduce the error

Generate a new project using the JDL provided in "JHipster configuration" section, start the project, go for instance to “Albums” entity and click the arrow to sort by “Artist”.

Related issues

n.a.

Suggest a Fix

As to my knowledge, we cloud change the EntityManager component to detect “.” and correct the order field as follow.

package com.jhipster.demo.bootifulmusic.repository;

(...)

/**
 * Helper class to create SQL selects based on the entity, paging parameters and criteria.
 *
 */
@Component
public class EntityManager {
    (...)
    private static Collection<? extends OrderByField> createOrderByFields(Table table, Sort sortToUse) {
        List<OrderByField> fields = new ArrayList<>();

        for (Sort.Order order : sortToUse) {
            String propertyName = order.getProperty();
//             OrderByField orderByField = OrderByField.from(table.column(propertyName).as(EntityManager.ALIAS_PREFIX + propertyName));
            // Begin fix
            OrderByField orderByField = !propertyName.contains(".") ?
                OrderByField.from(table.column(propertyName).as(EntityManager.ALIAS_PREFIX + propertyName))
                :   createOrderByField(propertyName);
            // End fix
            fields.add(order.isAscending() ? orderByField.asc() : orderByField.desc());
        }

        return fields;
    }

    /**
     * Creates an OrderByField instance for sorting a query by the specified property.
     *
     * @param propertyName The full property name in the format "tableName.columnName".
     * @return An OrderByField instance for sorting by the specified property.
     */
    private static OrderByField createOrderByField(String propertyName) {
        // Split the propertyName into table name and column name
        String[] parts = propertyName.split("\\.");
        String tableName = parts[0];
        String columnName = parts[1];

        // Create and return an OrderByField instance
        return OrderByField.from(
            // Create a column with the given name and alias it with the table name and column name
            Column.aliased(columnName,
                // Create a table alias with the same name as the table
                Table.aliased(camelCaseToSnakeCase(tableName), tableName),
                // Use a composite alias of "tableName_columnName"
                String.format("%s_%s", tableName, columnName)
            )
        );
    }

    /**
     * Converts a camel case string to snake case.
     *
     * @param input The camel case string to be converted to snake case.
     * @return The input string converted to snake case.
     */
    public static String camelCaseToSnakeCase(String input) {
        // Regular Expression
        String regex = "([a-z])([A-Z]+)";

        // Replacement string
        String replacement = "$1_$2";

        // Replace the given regex
        // with replacement string
        // and convert it to lower case.
        return input
            .replaceAll(
                regex, replacement)
            .toLowerCase();
    }
}

Just saying, maybe it could be better...

JHipster Version(s)

Release 7.9.4 (2023-09-05) this is only affecting the reactive stack.

JHipster configuration

The following JDL can be use to demonstrate the issue

JDL definitions
application {
  config {
    baseName bootifulmusic,
    reactive true
    applicationType monolith
    authenticationType jwt
    buildTool maven
    packageName com.jhipster.demo.bootifulmusic,
    clientFramework vue
    prodDatabaseType mssql,
    enableTranslation true
    nativeLanguage pt-pt
    languages [en, pt-pt]
  }
  entities *
}

entity Artist {
    name String required
}

entity Genre {
    name String required
}

entity Track {
    name String required
}

entity Album {
    name String required
}

relationship OneToOne {
    Album{artist(name)} to Artist
    Album{genre(name)} to Genre
}

relationship OneToMany {
    Album{track(name)} to Track{album(name)}
}

paginate Artist, Genre, Track, Album with pagination
  
Entity configuration(s) entityName.json files generated in the .jhipster directory

n.a.

Browsers and Operating System

n.a.

mshima commented 12 months ago

@rjbgaspar can you contribute with a PR?

rjbgaspar commented 11 months ago

Hi @mshima,

I would like to, but unfortunately I don't know the generator project. I've said it before, is there any documentation that I can learn so that it can help in the future? I have taken a lot from the project and would also like to contribute.

mshima commented 11 months ago

Documentation is been rewritten in https://github.com/jhipster/generator-jhipster/pull/23305. Any suggestion is appreciated.

mraible commented 11 months ago

@rjbgaspar Here's one way that we try to make it easy to contribute:

  1. Generate a JHipster project
  2. Push it to GH on a main branch
  3. Create a new branch and push it; create a PR
  4. Create an issue in the generator-jhipster project, and tag @mraible
  5. If you want faster help, tag @deepu105
  6. Why deepu? He's one of the nicest guys ever.

Please let us know if you have suggestions for improvement!

rjbgaspar commented 11 months ago

Hi @mraible

Done it here.

mraible commented 11 months ago

Hello, @rjbgaspar. You need to change the destination of your PR from your fork to the main generator-jhipster project so all our CI tests will run.

rjbgaspar commented 10 months ago

Sory @mraible, @deepu105 need help!

I could figure out how to do it,

This is what i have tried after generating the project and push it to GH main branch as your instructions

git remote add upstream git@github.com:jhipster/generator-jhipster.git git branch fix-sorting-foreign-key
git checkout fix-sorting-foreign-key cp ../back/EntityManager.java src/main/java/com/jhipster/demo/bootifulmusic/repository <-- The file with the changes git add . git commit -m "fix bad SQL grammar is thrown when sorting using foreign key" git remote add upstream git@github.com:jhipster/generator-jhipster.git git remote -v

Output

origin  git@github.com:rjbgaspar/jhi-sorting-foreign-key.git (fetch)
origin  git@github.com:rjbgaspar/jhi-sorting-foreign-key.git (push)
upstream        git@github.com:jhipster/generator-jhipster.git (fetch)
upstream        git@github.com:jhipster/generator-jhipster.git (push)

git fetch upstream # Sync the Fork

Go into GH and create the PR, i do not see the remote repository to link the pull request Tanks In Advance

mraible commented 10 months ago

cp ../back/EntityManager.java src/main/java/com/jhipster/demo/bootifulmusic/repository <-- The file with the changes

This is not the path for the file that needs changes. The path is generators/spring-data-relational/templates/src/main/java/_package_/repository/EntityManager_reactive.java.ejs.

You can see the current version here.