Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rjbgaspar opened this issue Oct 14, 2023 · 8 comments · Fixed by #24340
Closed

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

rjbgaspar opened this issue Oct 14, 2023 · 8 comments · Fixed by #24340

Comments

@rjbgaspar
Copy link
Contributor

rjbgaspar commented Oct 14, 2023

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
Copy link
Member

mshima commented Oct 16, 2023

@rjbgaspar can you contribute with a PR?

@rjbgaspar
Copy link
Contributor Author

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
Copy link
Member

mshima commented Oct 26, 2023

Documentation is been rewritten in #23305.
Any suggestion is appreciated.

@mraible
Copy link
Contributor

mraible commented Nov 3, 2023

@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
Copy link
Contributor Author

Hi @mraible

Done it here.

@mraible
Copy link
Contributor

mraible commented Nov 13, 2023

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
Copy link
Contributor Author

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 [email protected]: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 [email protected]:jhipster/generator-jhipster.git
git remote -v

Output

origin  [email protected]:rjbgaspar/jhi-sorting-foreign-key.git (fetch)
origin  [email protected]:rjbgaspar/jhi-sorting-foreign-key.git (push)
upstream        [email protected]:jhipster/generator-jhipster.git (fetch)
upstream        [email protected]: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
Copy link
Contributor

mraible commented Nov 21, 2023

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.

rjbgaspar added a commit to rjbgaspar/generator-jhipster that referenced this issue Nov 22, 2023
mraible pushed a commit that referenced this issue Nov 25, 2023
@deepu105 deepu105 added this to the 8.1.0 milestone Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants