I have a method that builds the add query into a postgres db. The problem is that these queries seem to be case sensitive in addition to having to do a lot of formatting to build this query, and I was wondering if there was a simpler way to write this code.

@Transactional
    @NonNull
    public ImportFileActivityOutput processAdd(ImportFileActivityInput input) {
        try {
            importFileConverter.convertImportFile(input.file(), input.requiredColumns(), input.delimiter())
                .subscribe(
                    batch -> {
                        final var columnNames = new ArrayList<>(batch.get(0).keySet());
                        final var formattedColumnNames = columnNames.stream().map(col -> "\"" + col + "\"").collect(Collectors.joining(", "));
                        final var queryPlaceholders = columnNames.stream().map(col -> "?").collect(Collectors.joining(", "));
                        final var conflictColumns = input.primaryKeys().stream().map(pk -> "\"" + pk + "\"").collect(Collectors.joining(", ", "(", ")"));

                        final var insertQuery = String.format(
                            "INSERT INTO \"%s\".\"%s\" (%s) VALUES (%s) ON CONFLICT %s DO NOTHING",
                            SCHEMA, input.tableName(), formattedColumnNames, queryPlaceholders, conflictColumns
                        );

                        List<Object[]> batchArgs = batch.stream().map(row -> columnNames.stream().map(row::get).toArray(Object[]::new)).toList();
                        jdbcWrapper.executeBatchUpdate(insertQuery, batchArgs);
                    },
                    error -> {
                        log.error("Error during batch insertion for table {}: {}", input.tableName(), error.getMessage(), error);
                        throw new RuntimeException("Failed to process batch insert", error);
                    },
                    () -> log.info("Completed batch insertion for table: {}", input.tableName())
                );

            return new ImportFileActivityOutput(input.automationId(), ActivityStatus.SUCCESS, "add complete for table: " + input.tableName());

        } catch (Exception e) {
            log.error("Error during add operation for table {}: {}", input.tableName(), e.getMessage(), e);
            return new ImportFileActivityOutput(input.automationId(), ActivityStatus.FAILURE, "Add failed: " + e.getMessage());
        }
    }

public void executeBatchUpdate(String query, List<Object[]> batchArgs) {
        try {
            jdbcTemplate.batchUpdate(query, batchArgs);
        } catch (Exception e) {
            log.error("Error executing batch update: {}", e.getMessage(), e);
            throw new RuntimeException("Database operation failed", e);
        }
    }

convertImportFile returns something like [{id: id1, name: name1, job: job1},{id: id2, name: name2, job: job2}]

It just seems like it takes a lot of work to write out this query, with the black slashes, adding the ? and etc. are there some built in methods or something to simplify this process?

Btw is there anything to improve upon in this code in general? Does it make sense to create a separate function for the jdbc operations rather than stuffing it all inside processAdd()?

I have a method that builds the add query into a postgres db. The problem is that these queries seem to be case sensitive in addition to having to do a lot of formatting to build this query, and I was wondering if there was a simpler way to write this code.

@Transactional
    @NonNull
    public ImportFileActivityOutput processAdd(ImportFileActivityInput input) {
        try {
            importFileConverter.convertImportFile(input.file(), input.requiredColumns(), input.delimiter())
                .subscribe(
                    batch -> {
                        final var columnNames = new ArrayList<>(batch.get(0).keySet());
                        final var formattedColumnNames = columnNames.stream().map(col -> "\"" + col + "\"").collect(Collectors.joining(", "));
                        final var queryPlaceholders = columnNames.stream().map(col -> "?").collect(Collectors.joining(", "));
                        final var conflictColumns = input.primaryKeys().stream().map(pk -> "\"" + pk + "\"").collect(Collectors.joining(", ", "(", ")"));

                        final var insertQuery = String.format(
                            "INSERT INTO \"%s\".\"%s\" (%s) VALUES (%s) ON CONFLICT %s DO NOTHING",
                            SCHEMA, input.tableName(), formattedColumnNames, queryPlaceholders, conflictColumns
                        );

                        List<Object[]> batchArgs = batch.stream().map(row -> columnNames.stream().map(row::get).toArray(Object[]::new)).toList();
                        jdbcWrapper.executeBatchUpdate(insertQuery, batchArgs);
                    },
                    error -> {
                        log.error("Error during batch insertion for table {}: {}", input.tableName(), error.getMessage(), error);
                        throw new RuntimeException("Failed to process batch insert", error);
                    },
                    () -> log.info("Completed batch insertion for table: {}", input.tableName())
                );

            return new ImportFileActivityOutput(input.automationId(), ActivityStatus.SUCCESS, "add complete for table: " + input.tableName());

        } catch (Exception e) {
            log.error("Error during add operation for table {}: {}", input.tableName(), e.getMessage(), e);
            return new ImportFileActivityOutput(input.automationId(), ActivityStatus.FAILURE, "Add failed: " + e.getMessage());
        }
    }

public void executeBatchUpdate(String query, List<Object[]> batchArgs) {
        try {
            jdbcTemplate.batchUpdate(query, batchArgs);
        } catch (Exception e) {
            log.error("Error executing batch update: {}", e.getMessage(), e);
            throw new RuntimeException("Database operation failed", e);
        }
    }

convertImportFile returns something like [{id: id1, name: name1, job: job1},{id: id2, name: name2, job: job2}]

It just seems like it takes a lot of work to write out this query, with the black slashes, adding the ? and etc. are there some built in methods or something to simplify this process?

Btw is there anything to improve upon in this code in general? Does it make sense to create a separate function for the jdbc operations rather than stuffing it all inside processAdd()?

• java
• postgresql
• jdbc

Share Improve this question Follow asked Mar 3 at 14:39 geegee 5711 bronze badge 6

• 1 Your approach doesn't make much sense. Columns and table names are constants. This approach is only used when dynamically forming where clauses. – mr mcwolf Commented Mar 3 at 14:52
• @mrmcwolf but columns is dependent on what is returned by convertImportFile, table name is dependent on the ImportFileActivityInput? Right? This method will be used on a variety of table names where the columns and stuff will be different every time. It is all dependent on the input and what convertImportFile returns – gee Commented Mar 3 at 15:06
• Theoretically yes, but in practice the tables (with their columns) are pre-created in the database. If your goal is just to dynamically populate different tables depending on the object you are passing, use polymorphism. – mr mcwolf Commented Mar 3 at 15:13
• From what I can see, it seems like Android is involved, and you are trying to recreate the behavior of SQLiteDatabase. What are you actually trying to achieve? – mr mcwolf Commented Mar 3 at 15:16
• @mrmcwolf We dynamically determine the table name and schema by reading metadata files stored in S3 on a scheduled basis. Based on this metadata, we retrieve the corresponding CSV file from S3 and convert it into a list of hashmaps using convertImportFile(). The metadata provides all necessary details about the table structure, as nothing is predefined in the application. Using this information, we insert the data into a pre-existing PostgreSQL table. There are hundreds of tables and their schemas are only known from reading the metadata – gee Commented Mar 3 at 15:20

 |  Show 1 more comment

2 Answers 2

Sorted by: Reset to default 2

Your implementation leaves you vulnerable to sql injection attacks

You are "meta-crafting". So, no, there isn't 'a better way', and it looks weird because this isn't really what jdbc-template is for.

JDBC template is for making it easier (relative to raw JDBC, at least) to do DB queries, when the data is variable.

Data are things like:

• gee
• 29231999
• 2025-03-03 14:39:02Z
• 'How to simplify writing postgres querys with jdbctemplate?'

And you might want to insert these things into a table, where the code that does this can 'hardcode' the column names and table names.

But that isn't what you're doing here. You don't have data. Instead, you have definitions, or meta-data. You have table names and column names. Your inputs are things like:

• table Users
• table Questions
• column so_alias
• column so_userid
• column question_timestamp
• column title

and you want to craft SQL that is based on these inputs.

jdbc-template isn't for this. For example, most DB JDBC drivers don't let you use the ? mechanism to 'fill these in', and in that sense if any of your column or table names are sourced from potentially untrusted user input, you need to be careful because your code is extremely susceptible to SQL attacks, and the code you wrote in the post would be a security leak. I'm guessing the source of e.g. the columnNames collection is 'trusted' and this isn't an issue, but the javadoc of this method should really scream that out to the caller, that they MUST ensure it is trusted or this code means any hacker will own the database, in its entirety.

What you're doing is essentially writing a JDBC-template esque library, specifically one that can arrange for table dumps / backups / recovery of those backups given as inputs 'table names and column names'. The code of jdbc-template, JDBI, JOOQ, etc look... exactly like your code looks.

If you want it to look 'less bad', you'd.. look around, check if you aren't reinventing a wheel here. The concept of 'just dump all tables with all columns values' surely has come up before. Or, find alternate ways to do what you want. For example, instead of running table dumps, maybe set up your DB to duplicate its WAL to another server so that you have a backup server that is as up to date as the main one, continuously, and can even 'hot-replace' the main server if the main server goes down. Less data, and far better uptime if failover is needed / far better 'recency' if backups are needed (any new data is backed up within a second, instead of having to wait for the nightly dump). Once you have that, it obviates the need for doing this in code.

FWIW, jdbc-template buys you next to nothing here, you might as well write it straight on top of JDBC and eliminate a middle-man dependency - a dependency that doesn't really address the thing you are doing in the first place.