Conversation
📝 WalkthroughWalkthroughDisables Flyway validation on migrate for four datasource beans and centralizes guarded migration logic with repair-and-retry handling. Many SQL migrations were converted to idempotent conditional DDL (INFORMATION_SCHEMA checks / dynamic PREPARE/EXECUTE) and several new/modified migration scripts were added. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant FM as FlywayMigrator
participant FW as Flyway
participant DB as Database
App->>FM: migrate()
loop per-database
FM->>FM: migrateDatabase(flyway, dbName) / log start
FM->>FW: flyway.migrate()
FW->>DB: apply migrations
alt migrations succeed
FW-->>FM: success
FM->>FM: log success
else FlywayValidateException
FW-->>FM: validation error
FM->>FM: log validation error
FM->>FW: flyway.repair()
FW->>DB: repair metadata
FW-->>FM: repair complete
FM->>FW: flyway.migrate() (retry)
FW->>DB: apply migrations
FW-->>FM: success / failure
FM->>FM: log final outcome
end
end
FM->>App: return / rethrow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java (3)
12-12: Declareloggerasstatic finalNon-static logger fields create a separate instance per object. Standard SLF4J practice is
private static final Logger.♻️ Proposed fix
-private Logger logger = LoggerFactory.getLogger(FlywayMigrator.class); +private static final Logger logger = LoggerFactory.getLogger(FlywayMigrator.class);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java` at line 12, The logger field in FlywayMigrator is declared as an instance field; change the declaration of logger (Logger logger in class FlywayMigrator) to a static final field (private static final Logger) so a single SLF4J logger instance is used per class; update the declaration only (no behavior change) and ensure imports remain valid.
45-56: Use SLF4J parameterized logging instead of string concatenationString concatenation evaluates eagerly regardless of whether the log level is enabled, causing unnecessary allocations on hot paths.
♻️ Proposed refactor
-logger.info("Starting migration for database: " + dbName); +logger.info("Starting migration for database: {}", dbName); -logger.info("Successfully migrated database: " + dbName); +logger.info("Successfully migrated database: {}", dbName); -logger.warn("Validation error for " + dbName + ": " + e.getMessage() + ". Attempting repair..."); +logger.warn("Validation error for {}: {}. Attempting repair...", dbName, e.getMessage()); -logger.info("Repair completed for " + dbName + ". Retrying migration..."); +logger.info("Repair completed for {}. Retrying migration...", dbName); -logger.info("Successfully migrated database after repair: " + dbName); +logger.info("Successfully migrated database after repair: {}", dbName); -logger.error("Failed to repair and migrate " + dbName + ": " + repairException.getMessage(), repairException); +logger.error("Failed to repair and migrate {}: {}", dbName, repairException.getMessage(), repairException);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java` around lines 45 - 56, Replace all string-concatenation logging calls in FlywayMigrator (e.g., logger.info("Starting migration for database: " + dbName), logger.warn("Validation error for " + dbName + ": " + e.getMessage() + ". Attempting repair..."), logger.info("Repair completed for " + dbName + ". Retrying migration..."), logger.error("Failed to repair and migrate " + dbName + ": " + repairException.getMessage(), repairException)) with SLF4J parameterized logging (use placeholders like {} and pass dbName and exception objects as arguments) so message construction is deferred when the log level is disabled and exceptions are logged via the throwable parameter rather than concatenating getMessage().
31-34: Fail-fast cascade: a single DB migration failure silently skips all remaining databasesIf
migrateDatabase(flywayDbiemr, ...)throws, execution never reachesflywayDbidentity,flywayDbreporting, orflywayDb1097identity. In environments where one schema has stale state but others are healthy, this leaves every downstream database un-migrated on that startup cycle, potentially widening the outage scope.Consider whether a collect-all-errors approach (migrate all four regardless of earlier failures, then throw an aggregate exception) better suits the deployment requirements here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java` around lines 31 - 34, The current sequential calls in FlywayMigrator (migrateDatabase(flywayDbiemr, "db_iemr"), migrateDatabase(flywayDbidentity, "db_identity"), migrateDatabase(flywayDbreporting, "db_reporting"), migrateDatabase(flywayDb1097identity, "db_1097_identity")) fail fast and abort remaining migrations on the first exception; change this to a collect-all-errors pattern: invoke migrateDatabase for each of the four Flyway instances (flywayDbiemr, flywayDbidentity, flywayDbreporting, flywayDb1097identity) inside individual try/catch blocks, accumulate any thrown Exceptions (or their messages) into a list, and after attempting all four, if the list is non-empty throw an aggregated exception (or a new RuntimeException containing all messages) so callers see every failure rather than only the first. Ensure you reference the migrateDatabase method and preserve existing logging behavior while adding the aggregation/throw at the end.src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql (1)
1-10: Remove leftover dead code — duplicateUSEand commented-out legacy statements.Lines 1–6 are remnants of the pre-refactoring version of this file. Line 1's
use db_iemr;and the commented-outALTER TABLEstatements at lines 3–6 are superseded entirely by the new guarded block starting at line 10.🧹 Proposed cleanup
-use db_iemr; - --- ALTER TABLE t_kmfilemanager ADD COLUMN SubCategoryID INT NULL; - --- ALTER TABLE t_kmfilemanager ADD CONSTRAINT FK_KmFileManager_SubCategory --- FOREIGN KEY (SubCategoryID) REFERENCES m_subcategory(SubCategoryID); - - - USE db_iemr;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql` around lines 1 - 10, Remove the leftover duplicate "USE db_iemr;" and the commented-out legacy ALTER TABLE statements for t_kmfilemanager (the commented lines referencing SubCategoryID, FK_KmFileManager_SubCategory and m_subcategory); keep only the intended guarded/updated migration block and delete those dead lines so the file contains one USE statement and no obsolete commented schema changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/db/piramalswasthya/config/FlywayConfig.java`:
- Line 18: Remove the validateOnMigrate(false) call from all Flyway bean
builders in FlywayConfig (the beans for flyway, flywayDbidentity,
flywayDbreporting, and flywayDb1097identity) so Flyway's validation runs during
migrate() and allows FlywayValidateException to be thrown and handled by
FlywayMigrator.migrateDatabase; if you need to tolerate specific
missing/modified scripts instead of disabling validation entirely, configure
ignoreMigrationPatterns on the respective Flyway bean(s) rather than calling
validateOnMigrate(false).
In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java`:
- Line 35: In FlywayMigrator remove the debug System.out.println("SUCCESS");
statement (it duplicates the existing logger.info call) so only the logger is
used for success messages; locate the println in the FlywayMigrator class (near
the method performing migration) and delete that line, leaving the existing
logger.info(...) intact for consistent logging.
In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql`:
- Around line 13-17: The guard queries that check information_schema hardcode
table_schema = 'db_iemr', which breaks idempotency if the migration runs in a
differently named database; update the two checks that reference
information_schema (the one checking column_name = 'SubCategoryID' on table
t_kmfilemanager and the FK existence check) to use DATABASE() instead of the
literal 'db_iemr' so they inspect the current schema dynamically, keeping the
rest of the logic (t_kmfilemanager, SubCategoryID, and the FK name checks)
unchanged.
- Line 21: Replace the fallback SQL literals that use double quotes with
single-quoted strings escaped for embedding: find the occurrences of the literal
'SELECT "Column SubCategoryID already exists";' (and the similar fallback at the
other occurrence) in V9__1097Subcategorychanges.sql and change them to use
escaped single quotes for the inner string (i.e., embed single quotes by
doubling them) so the PREPARE/EXECUTE path becomes safe under ANSI_QUOTES;
update both occurrences accordingly.
---
Nitpick comments:
In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java`:
- Line 12: The logger field in FlywayMigrator is declared as an instance field;
change the declaration of logger (Logger logger in class FlywayMigrator) to a
static final field (private static final Logger) so a single SLF4J logger
instance is used per class; update the declaration only (no behavior change) and
ensure imports remain valid.
- Around line 45-56: Replace all string-concatenation logging calls in
FlywayMigrator (e.g., logger.info("Starting migration for database: " + dbName),
logger.warn("Validation error for " + dbName + ": " + e.getMessage() + ".
Attempting repair..."), logger.info("Repair completed for " + dbName + ".
Retrying migration..."), logger.error("Failed to repair and migrate " + dbName +
": " + repairException.getMessage(), repairException)) with SLF4J parameterized
logging (use placeholders like {} and pass dbName and exception objects as
arguments) so message construction is deferred when the log level is disabled
and exceptions are logged via the throwable parameter rather than concatenating
getMessage().
- Around line 31-34: The current sequential calls in FlywayMigrator
(migrateDatabase(flywayDbiemr, "db_iemr"), migrateDatabase(flywayDbidentity,
"db_identity"), migrateDatabase(flywayDbreporting, "db_reporting"),
migrateDatabase(flywayDb1097identity, "db_1097_identity")) fail fast and abort
remaining migrations on the first exception; change this to a collect-all-errors
pattern: invoke migrateDatabase for each of the four Flyway instances
(flywayDbiemr, flywayDbidentity, flywayDbreporting, flywayDb1097identity) inside
individual try/catch blocks, accumulate any thrown Exceptions (or their
messages) into a list, and after attempting all four, if the list is non-empty
throw an aggregated exception (or a new RuntimeException containing all
messages) so callers see every failure rather than only the first. Ensure you
reference the migrateDatabase method and preserve existing logging behavior
while adding the aggregation/throw at the end.
In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql`:
- Around line 1-10: Remove the leftover duplicate "USE db_iemr;" and the
commented-out legacy ALTER TABLE statements for t_kmfilemanager (the commented
lines referencing SubCategoryID, FK_KmFileManager_SubCategory and
m_subcategory); keep only the intended guarded/updated migration block and
delete those dead lines so the file contains one USE statement and no obsolete
commented schema changes.
| .dataSource(dataSource) | ||
| .locations("classpath:db/migration/dbiemr") | ||
| .baselineOnMigrate(true) | ||
| .validateOnMigrate(false) |
There was a problem hiding this comment.
validateOnMigrate(false) silently disables checksum integrity AND makes the FlywayValidateException repair logic in FlywayMigrator unreachable dead code
FlywayValidateException is thrown inside migrate() only when the internal doValidate() step runs — which is gated on validateOnMigrate. With it set to false on all four beans, flyway.migrate() will never throw FlywayValidateException, so the entire catch (org.flywaydb.core.api.exception.FlywayValidateException e) block in FlywayMigrator.migrateDatabase() — including the repair-and-retry path — is dead code that will never execute.
Additionally, validateOnMigrate verifies each migration's checksum to confirm it has not changed after being applied to the database. Disabling it permanently means a tampered or accidentally modified applied script will silently pass at startup.
The intent of this PR is clearly to repair-and-retry on validation failure. The correct approach is to remove validateOnMigrate(false) so that validation runs normally during migrate(), allowing FlywayValidateException to be thrown and caught by the handler in FlywayMigrator. If certain known-missing scripts need to be tolerated without a full disable, prefer ignoreMigrationPatterns.
🐛 Proposed fix — remove validateOnMigrate(false) from all four beans
return Flyway.configure()
.dataSource(dataSource)
.locations("classpath:db/migration/dbiemr")
.baselineOnMigrate(true)
- .validateOnMigrate(false)
.load();Apply the same removal to flywayDbidentity, flywayDbreporting, and flywayDb1097identity.
Also applies to: 28-28, 38-38, 48-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/db/piramalswasthya/config/FlywayConfig.java` at line 18,
Remove the validateOnMigrate(false) call from all Flyway bean builders in
FlywayConfig (the beans for flyway, flywayDbidentity, flywayDbreporting, and
flywayDb1097identity) so Flyway's validation runs during migrate() and allows
FlywayValidateException to be thrown and handled by
FlywayMigrator.migrateDatabase; if you need to tolerate specific
missing/modified scripts instead of disabling validation entirely, configure
ignoreMigrationPatterns on the respective Flyway bean(s) rather than calling
validateOnMigrate(false).
| migrateDatabase(flywayDbidentity, "db_identity"); | ||
| migrateDatabase(flywayDbreporting, "db_reporting"); | ||
| migrateDatabase(flywayDb1097identity, "db_1097_identity"); | ||
| System.out.println("SUCCESS"); |
There was a problem hiding this comment.
Remove debug System.out.println
Line 35 is a debug artifact that duplicates the logger.info on line 36.
🐛 Proposed fix
- System.out.println("SUCCESS");
logger.info("Flyway migration completed successfully");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| System.out.println("SUCCESS"); | |
| logger.info("Flyway migration completed successfully"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/db/piramalswasthya/config/FlywayMigrator.java` at line 35,
In FlywayMigrator remove the debug System.out.println("SUCCESS"); statement (it
duplicates the existing logger.info call) so only the logger is used for success
messages; locate the println in the FlywayMigrator class (near the method
performing migration) and delete that line, leaving the existing
logger.info(...) intact for consistent logging.
| SELECT COUNT(*) INTO @col_exists | ||
| FROM information_schema.columns | ||
| WHERE table_schema = 'db_iemr' | ||
| AND table_name = 't_kmfilemanager' | ||
| AND column_name = 'SubCategoryID'; |
There was a problem hiding this comment.
Hardcoded table_schema = 'db_iemr' makes the guard brittle.
Both information_schema checks (lines 13–17 for the column, lines 28–33 for the FK) hardcode the schema name as 'db_iemr'. If this migration ever runs against an instance where the schema is named differently (e.g., a dev or staging environment with a different naming convention), the check will always return 0, causing the migration to unconditionally attempt the DDL. On a second run this would fail with a "duplicate column" or "duplicate key name" error, defeating the idempotency goal.
Use DATABASE() to reference the current schema dynamically instead:
♻️ Proposed fix using `DATABASE()`
SELECT COUNT(*) INTO `@col_exists`
FROM information_schema.columns
-WHERE table_schema = 'db_iemr'
+WHERE table_schema = DATABASE()
AND table_name = 't_kmfilemanager'
AND column_name = 'SubCategoryID'; SELECT COUNT(*) INTO `@fk_exists`
FROM information_schema.table_constraints
-WHERE table_schema = 'db_iemr'
+WHERE table_schema = DATABASE()
AND table_name = 't_kmfilemanager'
AND constraint_type = 'FOREIGN KEY'
AND constraint_name = 'FK_KmFileManager_SubCategory';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT COUNT(*) INTO @col_exists | |
| FROM information_schema.columns | |
| WHERE table_schema = 'db_iemr' | |
| AND table_name = 't_kmfilemanager' | |
| AND column_name = 'SubCategoryID'; | |
| SELECT COUNT(*) INTO `@col_exists` | |
| FROM information_schema.columns | |
| WHERE table_schema = DATABASE() | |
| AND table_name = 't_kmfilemanager' | |
| AND column_name = 'SubCategoryID'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql` around
lines 13 - 17, The guard queries that check information_schema hardcode
table_schema = 'db_iemr', which breaks idempotency if the migration runs in a
differently named database; update the two checks that reference
information_schema (the one checking column_name = 'SubCategoryID' on table
t_kmfilemanager and the FK existence check) to use DATABASE() instead of the
literal 'db_iemr' so they inspect the current schema dynamically, keeping the
rest of the logic (t_kmfilemanager, SubCategoryID, and the FK name checks)
unchanged.
| SET @sql := IF( | ||
| @col_exists = 0, | ||
| 'ALTER TABLE t_kmfilemanager ADD COLUMN SubCategoryID INT NULL;', | ||
| 'SELECT "Column SubCategoryID already exists";' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MySQL ANSI_QUOTES sql_mode behavior double quotes PREPARE EXECUTE
💡 Result:
With sql_mode including ANSI_QUOTES, MySQL treats double quotes (") as identifier quotes (like backticks), not as string-literal quotes. That applies equally to statements executed normally and statements prepared via PREPARE/EXECUTE. So:
"col"means the identifiercol, not the string"col"; string literals must use single quotes ('...'). [1][2]- Example (fails under
ANSI_QUOTESbecause"abc"is parsed as an identifier):Correct form:SET SESSION sql_mode = 'ANSI_QUOTES'; PREPARE s FROM 'SELECT "abc"'; -- interpreted as SELECT <identifier abc> EXECUTE s; -- typically errors: unknown column 'abc'
PREPARE s FROM 'SELECT ''abc'''; -- or: PREPARE s FROM "SELECT 'abc'";
When does it take effect for prepared statements?
The SQL text is parsed/compiled at PREPARE time (for example, parameter types are determined when first prepared and then retained across EXECUTE calls). So the session’s sql_mode (including ANSI_QUOTES) needs to be set before PREPARE for it to influence how double quotes are interpreted inside the prepared SQL text. [3]
Still need a literal " character inside a string under ANSI_QUOTES?
Put it inside a single-quoted string literal, e.g. 'He said "hi"' (the " is just a character there). [2]
Sources: MySQL identifier quoting / ANSI_QUOTES behavior [1], string literal rules under ANSI_QUOTES [2], PREPARE compilation-time behavior (parameters typed when first prepared) [3].
[1] MySQL Manual: Identifiers and ANSI_QUOTES (double quotes quote identifiers; can’t quote strings)
[2] MySQL Manual: String literals (double-quoted strings disabled by ANSI_QUOTES)
[3] MySQL Manual: PREPARE statement (statement is prepared once; parameter types determined at first prepare and retained)
🏁 Script executed:
# Check if the file exists and view the relevant lines
if [ -f "src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql" ]; then
echo "=== File found, displaying context around line 21 ==="
sed -n '15,25p' "src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql" | cat -n
echo ""
echo "=== File found, displaying context around line 40 ==="
sed -n '35,45p' "src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql" | cat -n
else
echo "File not found at expected path"
# Try to find it
find . -name "V9__1097Subcategorychanges.sql" 2>/dev/null
fiRepository: PSMRI/AMRIT-DB
Length of output: 1703
Update fallback strings to use escaped single quotes for ANSI_QUOTES compatibility.
The fallback path 'SELECT "Column SubCategoryID already exists";' becomes unsafe if the MySQL server runs with ANSI_QUOTES in its sql_mode. Under ANSI_QUOTES, double quotes are interpreted as identifier delimiters (not string delimiters), causing MySQL to parse "Column SubCategoryID already exists" as a column reference, which fails with an Unknown column error when executed via PREPARE/EXECUTE.
Single-quoted strings remain safe regardless of sql_mode settings. Apply the same fix to both fallback strings at lines 21 and 40:
Proposed fixes
SET `@sql` := IF(
`@col_exists` = 0,
'ALTER TABLE t_kmfilemanager ADD COLUMN SubCategoryID INT NULL;',
- 'SELECT "Column SubCategoryID already exists";'
+ 'SELECT ''Column SubCategoryID already exists'';'
); SET `@sql2` := IF(
`@fk_exists` = 0,
'ALTER TABLE t_kmfilemanager
ADD CONSTRAINT FK_KmFileManager_SubCategory
FOREIGN KEY (SubCategoryID)
REFERENCES m_subcategory(SubCategoryID);',
- 'SELECT "Foreign key FK_KmFileManager_SubCategory already exists";'
+ 'SELECT ''Foreign key FK_KmFileManager_SubCategory already exists'';'
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'SELECT "Column SubCategoryID already exists";' | |
| 'SELECT ''Column SubCategoryID already exists'';' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql` at
line 21, Replace the fallback SQL literals that use double quotes with
single-quoted strings escaped for embedding: find the occurrences of the literal
'SELECT "Column SubCategoryID already exists";' (and the similar fallback at the
other occurrence) in V9__1097Subcategorychanges.sql and change them to use
escaped single quotes for the inner string (i.e., embed single quotes by
doubling them) so the PREPARE/EXECUTE path becomes safe under ANSI_QUOTES;
update both occurrences accordingly.
* fix:changes in face embedding sql file version 2 * fix:changes sql file for migratio * fix:changes for all migrationsql * fix:new final fixes for deployement * fix:changes on 28thNov * fix:new changes 30/11/2025
* fix:changes in face embedding sql file version 2 * fix:changes sql file for migratio * fix:changes for all migrationsql * fix:new final fixes for deployement * fix:changes on 28thNov * fix:new changes 30/11/2025 * fix:new changes part-2 30/11/2025 * fix:new changes 02/12/2025 * fix:new changes part-2 02/12/2025 * fix:new changes part-3 02/12/2025 --------- Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
* fix:changes in face embedding sql file version 2 * fix:changes sql file for migratio * fix:changes for all migrationsql * fix:new final fixes for deployement * fix:changes on 28thNov * fix:new changes 30/11/2025 * fix:new changes part-2 30/11/2025 * fix:new changes 02/12/2025 * fix:new changes part-2 02/12/2025 * fix:new changes part-3 02/12/2025 * fix: new fixes for sanofi prod 5/12/2025 --------- Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
…ution time (PSMRI#37) * fix: optimize the query for yearly report * fix: rename the version to resolve the conflicts * fix: add the drop procedure * fix: version number * fix: coderabbit comments * fix: version change
* fix: update the query to fix the issue in datasync * fix: update the query to fix the issue in datasync * fix: update the version number * fix: remove unwanted file * fix: add syncfacilityID in ItemStockExit table * fix: remove the trim query * fix: version number * fix: version change
Co-authored-by: Mithun James <1007084+drtechie@users.noreply.github.com>
* fix:changes in face embedding sql file version 2 * fix:changes sql file for migratio * fix:changes for all migrationsql * fix:new final fixes for deployement * fix:changes on 28thNov * fix:new changes 30/11/2025 * fix:new changes part-2 30/11/2025 * fix:new changes 02/12/2025 * fix:new changes part-2 02/12/2025 * fix:new changes part-3 02/12/2025 * fix: new fixes for sanofi prod 5/12/2025 * fix:t_idrdetails column modification * fix:Modify table's column and added SP * fix:change in v_23 * fix:v28 file * fix:12/12/2025 * fix:query fixed acc. to coderabbit --------- Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
* fix: mda form and leprosy form added * fix: not exist * arrange the level table * fix:semicolon added
* fix:changes in face embedding sql file version 2 * fix:changes sql file for migratio * fix:changes for all migrationsql * fix:new final fixes for deployement * fix:changes on 28thNov * fix:new changes 30/11/2025 * fix:new changes part-2 30/11/2025 * fix:new changes 02/12/2025 * fix:new changes part-2 02/12/2025 * fix:new changes part-3 02/12/2025 * fix: new fixes for sanofi prod 5/12/2025 * fix:t_idrdetails column modification * fix:Modify table's column and added SP * fix:change in v_23 * fix:v28 file * fix:12/12/2025 * fix:query fixed acc. to coderabbit * fix:optimised stored procedure --------- Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
* add new column * fix coderabit comments * change column name * change column name
* AMM-2104,AMM-2105,AMM-2114 * fix coderabit comments * fix db campaign_filariasis_mda * fix sql * fix sql
* migration for release 3.7.0 * migration for release 3.7.0 * migration for release 3.7.0
* add new column in hbn table * fix
* add new column in hbn table * add migration for children column * add migration for children column
Co-authored-by: Mithun James <1007084+drtechie@users.noreply.github.com>
* add auto_increment on id feild * add auto_increment on id feild * add auto_increment on id feild * add auto_increment on id feild * remove duplicate db_iemr
|
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql (1)
1-10:⚠️ Potential issue | 🟡 MinorRemove the redundant
USE db_iemr;statement.Line 1 already contains
use db_iemr;. The newly addedUSE db_iemr;at line 10 is a duplicate and should be removed.🔧 Proposed fix
use db_iemr; -- ALTER TABLE t_kmfilemanager ADD COLUMN SubCategoryID INT NULL; -- ALTER TABLE t_kmfilemanager ADD CONSTRAINT FK_KmFileManager_SubCategory -- FOREIGN KEY (SubCategoryID) REFERENCES m_subcategory(SubCategoryID); -USE db_iemr; - -- 1) ADD COLUMN IF NOT EXISTS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V9__1097Subcategorychanges.sql` around lines 1 - 10, Remove the duplicate database selection: in V9__1097Subcategorychanges.sql there are two occurrences of the statement "USE db_iemr;"; delete the second one (the later "USE db_iemr;") so only the initial use db_iemr; remains and no redundant database switch is present.src/main/resources/db/migration/dbiemr/V13__QUALITY_MODULE_CATEGORY_MAPPING.sql (3)
33-37:⚠️ Potential issue | 🟠 MajorHardcoded
DEFINERwill causeERROR 1449on any environment wherepiramaldevdoesn't exist.
DEFINER = \piramaldev`@`%`is baked into the migration. TheCREATE VIEWstatement will succeed at migration time, but any subsequent query against the view will throwERROR 1449: The user specified as a definer does not existon environments (staging, prod, CI) where this specific MySQL user is absent. The stored procedure on line 67 correctly omits aDEFINER` clause — the same approach should be applied to the view.🐛 Proposed fix — drop the `DEFINER` clause so MySQL defaults to the current user
CREATE ALGORITHM = UNDEFINED - DEFINER = `piramaldev`@`%` SQL SECURITY DEFINER VIEW `db_iemr`.`v_get_qualityaudit_sectionquestionairevalues` AS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V13__QUALITY_MODULE_CATEGORY_MAPPING.sql` around lines 33 - 37, The CREATE VIEW statement for v_get_qualityaudit_sectionquestionairevalues includes a hardcoded DEFINER=`piramaldev`@`%` which will cause ERROR 1449 in environments where that user doesn't exist; remove the entire DEFINER = `piramaldev`@`%` (and any ALGORITHM/SQL SECURITY clauses if you prefer leaving only the portable CREATE VIEW ... AS ...) so the view is created without an explicit definer and will default to the current user, mirroring the stored procedure approach used later in this migration.
67-91:⚠️ Potential issue | 🟠 MajorMissing soft-delete filter — procedure may return deleted records.
The view counterpart (lines 56–59) explicitly guards all three joined tables with
Deleted IS FALSE. The stored procedure returnsq.deletedin its SELECT list (line 79) but never filters ons.Deletedorq.Deletedin the WHERE clause. Callers will receive logically deleted sections and questions, which is inconsistent with the view's contract.🐛 Proposed fix — add deleted guards matching the view's behavior
WHERE s.providerservicemapid = IFNULL(v_psmrid, s.providerservicemapid) AND s.providerservicemapid IS NOT NULL - AND s.id IS NOT NULL; + AND s.Deleted IS FALSE + AND q.Deleted IS FALSE;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V13__QUALITY_MODULE_CATEGORY_MAPPING.sql` around lines 67 - 91, The stored procedure Pr_QualityAuditorSectionQuestionaire returns rows from m_qualityauditsection (alias s) joined with m_qualityauditquestionnaire (alias q) but does not filter out soft-deleted rows; update the WHERE clause to mirror the view by adding guards for s.Deleted = FALSE and q.Deleted = FALSE (or IS FALSE) in addition to the existing providerservicemapid/NULL checks so logically deleted sections and questions are excluded.
15-19:⚠️ Potential issue | 🟡 MinorDouble-quoted string literal breaks under
ANSI_QUOTESSQL mode.
"Column already exists"on line 18 uses double quotes, which are treated as identifier delimiters (not string literals) whenANSI_QUOTESis enabled. This would cause the no-op fallback branch to fail with a syntax error.🐛 Proposed fix
SET `@sql` = IF( `@col_exists` = 0, 'ALTER TABLE m_qualityauditquestionnaire ADD COLUMN Role VARCHAR(50) DEFAULT ''Associate,ANM,MO'';', - 'SELECT "Column already exists";' + 'SELECT ''Column already exists'';' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V13__QUALITY_MODULE_CATEGORY_MAPPING.sql` around lines 15 - 19, The fallback branch of the IF assigns a double-quoted token ("Column already exists") which is an identifier under ANSI_QUOTES; update the no-op branch in the `@sql` assignment so it uses a proper string literal (single quotes) or an ANSI-safe expression (e.g., SELECT 'Column already exists') instead; modify the IF that sets `@sql` (referencing `@col_exists` and `@sql` in V13__QUALITY_MODULE_CATEGORY_MAPPING.sql) to replace the double-quoted value with a single-quoted string.src/main/resources/db/migration/dbiemr/V22__DB_IMER_incentive.sql (1)
40-44:⚠️ Potential issue | 🟡 MinorTypo in column names: "assame" should be "assamese".
Lines 40 and 43 contain
assame_activity_descriptionandassame_group_name, which should beassamese_activity_descriptionandassamese_group_name(referring to the Assamese language). No application code currently references these column names, making this a safe change to apply now before deployment.♻️ Suggested fix
- `assame_activity_description` VARCHAR(1000) DEFAULT NULL, + `assamese_activity_description` VARCHAR(1000) DEFAULT NULL, `hindi_activity_description` VARCHAR(1000) DEFAULT NULL, `activity_id` BIGINT DEFAULT NULL, - `assame_group_name` VARCHAR(200) DEFAULT NULL, + `assamese_group_name` VARCHAR(200) DEFAULT NULL,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/dbiemr/V22__DB_IMER_incentive.sql` around lines 40 - 44, The two column names `assame_activity_description` and `assame_group_name` are misspelled and should be renamed to `assamese_activity_description` and `assamese_group_name`; update the migration SQL to replace those identifiers (preserving types VARCHAR(1000)/VARCHAR(200) and DEFAULT NULL) and also check and update any related constraints, indexes, or references in the same migration (e.g., ALTER TABLE definitions or column lists) to use the corrected `assamese_*` names so the schema is consistent.
| declare v_NextAttemptPeriod int(11); | ||
| select distinct NextAttemptPeriod into v_NextAttemptPeriod from m_mctscallconfiguration | ||
| where current_date() between effectivefrom and EffectiveUpto | ||
| and OutboundCallType like '%ecd%' | ||
| and deleted is false; |
There was a problem hiding this comment.
v_NextAttemptPeriod will be NULL if no configuration row matches, causing silent empty results.
If the SELECT ... INTO finds no rows, v_NextAttemptPeriod remains NULL. All subsequent comparisons (v_NextAttemptPeriod=0, v_NextAttemptPeriod>0) evaluate to NULL, so the CASE in the WHERE clause (Lines 137–149) returns NULL for every row, and the procedure silently returns zero results with no error.
Add a guard after the SELECT INTO to handle the NULL case — either default to a sensible value or raise a signal:
Proposed fix
declare v_NextAttemptPeriod int(11);
select distinct NextAttemptPeriod into v_NextAttemptPeriod from m_mctscallconfiguration
where current_date() between effectivefrom and EffectiveUpto
and OutboundCallType like '%ecd%'
and deleted is false;
+
+IF v_NextAttemptPeriod IS NULL THEN
+ SET v_NextAttemptPeriod = 0;
+END IF;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| declare v_NextAttemptPeriod int(11); | |
| select distinct NextAttemptPeriod into v_NextAttemptPeriod from m_mctscallconfiguration | |
| where current_date() between effectivefrom and EffectiveUpto | |
| and OutboundCallType like '%ecd%' | |
| and deleted is false; | |
| declare v_NextAttemptPeriod int(11); | |
| select distinct NextAttemptPeriod into v_NextAttemptPeriod from m_mctscallconfiguration | |
| where current_date() between effectivefrom and EffectiveUpto | |
| and OutboundCallType like '%ecd%' | |
| and deleted is false; | |
| IF v_NextAttemptPeriod IS NULL THEN | |
| SET v_NextAttemptPeriod = 0; | |
| END IF; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V34__SP_ECD.sql` around lines 8 - 12,
v_NextAttemptPeriod can be NULL when the SELECT ... INTO from
m_mctscallconfiguration returns no rows, which makes subsequent comparisons in
the CASE (used in the WHERE clause around the v_NextAttemptPeriod=0 / >0 logic)
evaluate to NULL and produce empty results; after the SELECT INTO that populates
v_NextAttemptPeriod add a guard: check IF v_NextAttemptPeriod IS NULL THEN
either SET v_NextAttemptPeriod to a sensible default (e.g., 0 or another
business-appropriate value) or RAISE/ SIGNAL an error with a clear message so
the procedure does not silently return zero rows — update the code surrounding
v_NextAttemptPeriod and the CASE used in the WHERE to rely on this non-NULL
value.
| declare v_NextAttemptPeriod int(11); | ||
| select distinct NextAttemptPeriod into v_NextAttemptPeriod from m_mctscallconfiguration where current_date() between effectivefrom and EffectiveUpto | ||
| and OutboundCallType like '%ecd%' | ||
| and deleted is false; |
There was a problem hiding this comment.
Same NULL v_NextAttemptPeriod risk as V34 — apply the same guard here.
If no matching configuration row is found, v_NextAttemptPeriod will be NULL, and the CASE filter on Lines 128–136 will silently exclude all rows. Add the same IF v_NextAttemptPeriod IS NULL THEN SET v_NextAttemptPeriod = 0; END IF; guard recommended for V34.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V35__ECD_Child_SP.sql` around lines 9
- 12, The variable v_NextAttemptPeriod populated from m_mctscallconfiguration
can be NULL if no rows match; after the SELECT into v_NextAttemptPeriod add a
guard: check IF v_NextAttemptPeriod IS NULL THEN set v_NextAttemptPeriod = 0;
END IF; so downstream CASE filtering that uses v_NextAttemptPeriod (in this
procedure) does not silently exclude all rows.
| left join ( | ||
| -- select obcallid,max(createddate) max_createddate from t_bencall group by obcallid | ||
| select t1.obcallid,max(t1.createddate) max_createddate from t_bencall t1 | ||
| inner join t_MCTSOutboundCalls t2 on t1.obcallid=t2.obcallid where t1.CalledServiceID=1714 and AllocatedUserID = v_AllocatedUserID | ||
| group by t1.obcallid | ||
| ) b2 on b2.obcallid=mctsoutbou0_.obcallid | ||
| left join t_bencall b on b.obcallid=b2.obcallid and b.createddate=b2.max_createddate |
There was a problem hiding this comment.
Missing callendtime IS NOT NULL filter in subqueries — inconsistent with the mother procedure (V34).
In V34 (mother procedure), both the b2 subquery and the b join include filters ensuring only completed calls are considered:
b2:t1.callendtime is not null(V34 Line 127)bjoin:b.CalledServiceID=1714 and b.callendtime is not null(V34 Line 130)
In V35, both are missing:
- Line 118: no
t1.callendtime is not null - Line 121: no
b.CalledServiceID=1714 and b.callendtime is not null
This means the child procedure may match in-progress (incomplete) call records when computing isCallDisconnected and sort_createddate, leading to incorrect worklist ordering or stale data.
Proposed fix to align with V34
left join (
select t1.obcallid,max(t1.createddate) max_createddate from t_bencall t1
- inner join t_MCTSOutboundCalls t2 on t1.obcallid=t2.obcallid where t1.CalledServiceID=1714 and AllocatedUserID = v_AllocatedUserID
+ inner join t_MCTSOutboundCalls t2 on t1.obcallid=t2.obcallid where t1.CalledServiceID=1714 and t2.AllocatedUserID = v_AllocatedUserID and t1.callendtime is not null
group by t1.obcallid
) b2 on b2.obcallid=mctsoutbou0_.obcallid
- left join t_bencall b on b.obcallid=b2.obcallid and b.createddate=b2.max_createddate
+ left join t_bencall b on b.obcallid=b2.obcallid and b.createddate=b2.max_createddate and b.CalledServiceID=1714 and b.callendtime is not null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| left join ( | |
| -- select obcallid,max(createddate) max_createddate from t_bencall group by obcallid | |
| select t1.obcallid,max(t1.createddate) max_createddate from t_bencall t1 | |
| inner join t_MCTSOutboundCalls t2 on t1.obcallid=t2.obcallid where t1.CalledServiceID=1714 and AllocatedUserID = v_AllocatedUserID | |
| group by t1.obcallid | |
| ) b2 on b2.obcallid=mctsoutbou0_.obcallid | |
| left join t_bencall b on b.obcallid=b2.obcallid and b.createddate=b2.max_createddate | |
| left join ( | |
| -- select obcallid,max(createddate) max_createddate from t_bencall group by obcallid | |
| select t1.obcallid,max(t1.createddate) max_createddate from t_bencall t1 | |
| inner join t_MCTSOutboundCalls t2 on t1.obcallid=t2.obcallid where t1.CalledServiceID=1714 and t2.AllocatedUserID = v_AllocatedUserID and t1.callendtime is not null | |
| group by t1.obcallid | |
| ) b2 on b2.obcallid=mctsoutbou0_.obcallid | |
| left join t_bencall b on b.obcallid=b2.obcallid and b.createddate=b2.max_createddate and b.CalledServiceID=1714 and b.callendtime is not null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V35__ECD_Child_SP.sql` around lines
115 - 121, The b2 subquery and the subsequent join to alias b in
V35__ECD_Child_SP.sql are missing the "completed call" filter, causing
in-progress calls to be included; update the b2 subquery (alias t1 in t_bencall)
to include "t1.callendtime IS NOT NULL" in its WHERE clause (keeping the
existing CalledServiceID=1714 and AllocatedUserID = v_AllocatedUserID) and add
the same completion filter to the join condition for b (i.e., include
"b.CalledServiceID=1714 AND b.callendtime IS NOT NULL" when joining b on
b2.obcallid) so behavior matches V34's b2 and b filters and prevents incomplete
calls from affecting isCallDisconnected and sort_createddate.
| SET @table_exists = ( | ||
| SELECT COUNT(*) | ||
| FROM information_schema.TABLES | ||
| WHERE TABLE_SCHEMA = 'db_iemr' | ||
| AND TABLE_NAME = 't_cdr' | ||
| ); | ||
|
|
||
| -- Proceed only if table exists | ||
| SET @sql_check = IF( | ||
| @table_exists > 0, | ||
| 'SELECT "Table t_cdr exists. Proceeding with migration..." AS status', | ||
| 'SELECT "ERROR: Table t_cdr does not exist!" AS status' | ||
| ); | ||
| PREPARE stmt FROM @sql_check; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; | ||
|
|
||
| -- Add cdr_image_1 (LONGTEXT) | ||
| SET @column_exists_1 = ( | ||
| SELECT COUNT(*) | ||
| FROM information_schema.COLUMNS | ||
| WHERE TABLE_SCHEMA = 'db_iemr' | ||
| AND TABLE_NAME = 't_cdr' | ||
| AND COLUMN_NAME = 'cdr_image_1' | ||
| ); | ||
|
|
||
| SET @sql_add_column_1 = IF( | ||
| @column_exists_1 = 0, | ||
| 'ALTER TABLE db_iemr.t_cdr ADD COLUMN cdr_image_1 LONGTEXT DEFAULT NULL COMMENT "First CDR image (base64 encoded or file path)"', | ||
| 'SELECT "Column cdr_image_1 already exists, skipping..." AS status' | ||
| ); | ||
| PREPARE stmt FROM @sql_add_column_1; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; | ||
|
|
||
| -- Add cdr_image_2 (LONGTEXT) | ||
| SET @column_exists_2 = ( | ||
| SELECT COUNT(*) | ||
| FROM information_schema.COLUMNS | ||
| WHERE TABLE_SCHEMA = 'db_iemr' | ||
| AND TABLE_NAME = 't_cdr' | ||
| AND COLUMN_NAME = 'cdr_image_2' | ||
| ); | ||
|
|
||
| SET @sql_add_column_2 = IF( | ||
| @column_exists_2 = 0, | ||
| 'ALTER TABLE db_iemr.t_cdr ADD COLUMN cdr_image_2 LONGTEXT DEFAULT NULL COMMENT "Second CDR image (base64 encoded or file path)"', | ||
| 'SELECT "Column cdr_image_2 already exists, skipping..." AS status' | ||
| ); | ||
| PREPARE stmt FROM @sql_add_column_2; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; | ||
|
|
||
| -- Add death_cert_image_1 (LONGTEXT) | ||
| SET @column_exists_3 = ( | ||
| SELECT COUNT(*) | ||
| FROM information_schema.COLUMNS | ||
| WHERE TABLE_SCHEMA = 'db_iemr' | ||
| AND TABLE_NAME = 't_cdr' | ||
| AND COLUMN_NAME = 'death_cert_image_1' | ||
| ); | ||
|
|
||
| SET @sql_add_column_3 = IF( | ||
| @column_exists_3 = 0, | ||
| 'ALTER TABLE db_iemr.t_cdr ADD COLUMN death_cert_image_1 LONGTEXT DEFAULT NULL COMMENT "First death certificate image (base64 encoded or file path)"', | ||
| 'SELECT "Column death_cert_image_1 already exists, skipping..." AS status' | ||
| ); | ||
| PREPARE stmt FROM @sql_add_column_3; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; | ||
|
|
||
| -- Add death_cert_image_2 (LONGTEXT) | ||
| SET @column_exists_4 = ( | ||
| SELECT COUNT(*) | ||
| FROM information_schema.COLUMNS | ||
| WHERE TABLE_SCHEMA = 'db_iemr' | ||
| AND TABLE_NAME = 't_cdr' | ||
| AND COLUMN_NAME = 'death_cert_image_2' | ||
| ); | ||
|
|
||
| SET @sql_add_column_4 = IF( | ||
| @column_exists_4 = 0, | ||
| 'ALTER TABLE db_iemr.t_cdr ADD COLUMN death_cert_image_2 LONGTEXT DEFAULT NULL COMMENT "Second death certificate image (base64 encoded or file path)"', | ||
| 'SELECT "Column death_cert_image_2 already exists, skipping..." AS status' | ||
| ); | ||
| PREPARE stmt FROM @sql_add_column_4; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; |
There was a problem hiding this comment.
t_cdr block has no @proceed guard — migration will crash if the table is absent
The @table_exists check for t_cdr only emits a status message; it never sets a @proceed flag. If t_cdr does not exist:
@column_exists_Nevaluates to0(no rows inINFORMATION_SCHEMA.COLUMNSfor a missing table).- Each
IF(@column_exists_N= 0, 'ALTER TABLE ...', ...)resolves to theALTER TABLEstring. EXECUTE stmtfires anALTER TABLEagainst a non-existent table → MySQL Error 1146 → Flyway migration fails.
The t_mdsr block (line 110) correctly guards against this with SET @proceed = IF(@table_exists > 0, 1, 0). Apply the same pattern to t_cdr.
🐛 Proposed fix — add `@proceed` guard to the `t_cdr` block
PREPARE stmt FROM `@sql_check`;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;
+ -- Stop if table doesn't exist
+ SET `@proceed` = IF(`@table_exists` > 0, 1, 0);
+
-- Add cdr_image_1 (LONGTEXT)
SET `@column_exists_1` = (
SELECT COUNT(*)
FROM information_schema.COLUMNS
WHERE TABLE_SCHEMA = 'db_iemr'
AND TABLE_NAME = 't_cdr'
AND COLUMN_NAME = 'cdr_image_1'
);
SET `@sql_add_column_1` = IF(
- `@column_exists_1` = 0,
- 'ALTER TABLE db_iemr.t_cdr ADD COLUMN cdr_image_1 LONGTEXT DEFAULT NULL COMMENT "First CDR image (base64 encoded or file path)"',
- 'SELECT "Column cdr_image_1 already exists, skipping..." AS status'
+ `@column_exists_1` = 0 AND `@proceed` = 1,
+ 'ALTER TABLE db_iemr.t_cdr ADD COLUMN cdr_image_1 LONGTEXT DEFAULT NULL COMMENT \'First CDR image (base64 encoded or file path)\'',
+ IF(`@column_exists_1` > 0,
+ 'SELECT \'Column cdr_image_1 already exists, skipping...\' AS status',
+ 'SELECT \'Skipping due to table not found\' AS status'
+ )
);Apply the same pattern to cdr_image_2, death_cert_image_1, and death_cert_image_2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V45__cdr_mdr_dbmigtaion.sql` around
lines 3 - 90, Add a `@proceed` guard like the t_mdsr block: after computing
`@table_exists` for t_cdr, set SET `@proceed` = IF(`@table_exists` > 0, 1, 0) and then
change each column-add conditional to require `@proceed` = 1 as well (e.g. build
`@sql_add_column_1`..4 with IF(`@proceed` = 1 AND `@column_exists_N` = 0, 'ALTER TABLE
db_iemr.t_cdr ADD COLUMN ...', 'SELECT "Column ... already exists or table
missing, skipping..." AS status')) so PREPARE/EXECUTE stmt never runs an ALTER
TABLE when the table is absent; update all referenced variables
(`@column_exists_1`..4, `@sql_add_column_1`..4, PREPARE stmt, EXECUTE stmt)
accordingly.
| SET @preparedStatement = (SELECT IF( | ||
| (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = @dbname AND TABLE_NAME = @tablename AND COLUMN_NAME = @columnname) > 0, | ||
| 'SELECT 1', | ||
| CONCAT('ALTER TABLE ', @dbname, '.', @tablename, ' ADD COLUMN ', @columnname, ' VARCHAR(255) DEFAULT NULL;') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/resources/db/migration/dbiemr/V56__t_cbacdetails_migration.sqlRepository: PSMRI/AMRIT-DB
Length of output: 4292
🌐 Web query:
MySQL PREPARE statement trailing semicolon documented behavior
💡 Result:
In MySQL, the semicolon (;) is a client-side statement delimiter, not part of the SQL statement text sent to the server. The mysql CLI (and similar tools) treat ; as “end of statement,” and this behavior is controlled by the client’s delimiter setting (default ;).[2]
PREPARE stmt_name FROM preparable_stmt is different: the SQL to prepare is inside a string literal / user variable and is parsed by the server as statement text. The MySQL manual specifies that preparable_stmt “must represent a single statement, not multiple statements.”[1] A trailing semicolon inside the prepared SQL string therefore does not act as a delimiter; it’s an extra token in the statement text and commonly results in a syntax error (for example, PREPARE s FROM 'SELECT 1;'; fails, while PREPARE s FROM 'SELECT 1'; works), because the server expects the statement to end after the single statement.[1]
Sources
[1] MySQL Reference Manual – PREPARE statement (“single statement, not multiple statements”) https://dev.mysql.com/doc/refman/8.4/en/prepare.html
[2] MySQL Reference Manual – mysql client delimiter command (client interprets statement separator; default ;) https://dev.mysql.com/doc/refman/5.7/en/mysql-commands.html
Remove trailing semicolons from prepared statement strings — this will cause the migration to fail
Each CONCAT call on lines 11, 21, 31, 41, 51, 61, 71, and 81 embeds a trailing ; in the generated SQL string. According to the MySQL manual, a prepared statement string preparable_stmt "must represent a single statement, not multiple statements." A trailing semicolon inside the string is parsed as an extra token by the server, causing a syntax error when the prepared statement executes. For example, PREPARE stmt FROM 'SELECT 1;'; fails, while PREPARE stmt FROM 'SELECT 1'; succeeds. This inconsistency with the 'SELECT 1' no-op branch (which correctly omits any terminator) confirms the error.
🔴 Proposed fix — remove the trailing semicolons from all CONCAT statements
- CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, ' VARCHAR(255) DEFAULT NULL;')
+ CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, ' VARCHAR(255) DEFAULT NULL')Apply the same change to all eight CONCAT calls (lines 11, 21, 31, 41, 51, 61, 71, 81), including the TINYINT(1) variant on line 81.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CONCAT('ALTER TABLE ', @dbname, '.', @tablename, ' ADD COLUMN ', @columnname, ' VARCHAR(255) DEFAULT NULL;') | |
| CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, ' VARCHAR(255) DEFAULT NULL') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V56__t_cbacdetails_migration.sql` at
line 11, The CONCAT expressions used to build prepared statements (e.g.,
CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, '
VARCHAR(255) DEFAULT NULL;')) include a trailing semicolon which makes PREPARE
stmt fail; remove the trailing ';' from all CONCAT calls (including the
VARCHAR(255) and TINYINT(1) variants) so the generated preparable_stmt contains
no terminator, ensuring PREPARE/EXECUTE work correctly.
| SET @columnname = 'steilisation_date'; | ||
| SET @preparedStatement = (SELECT IF( | ||
| (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = @dbname AND TABLE_NAME = @tablename AND COLUMN_NAME = @columnname) > 0, | ||
| 'SELECT 1', | ||
| CONCAT('ALTER TABLE ', @dbname, '.', @tablename, ' ADD COLUMN ', @columnname, ' TIMESTAMP NULL;') | ||
| )); | ||
| PREPARE stmt FROM @preparedStatement; | ||
| EXECUTE stmt; | ||
| DEALLOCATE PREPARE stmt; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for any Java references to sterilisation/steilisation variants
rg -i 'steil[ia]sation|steil[ia]sation_date' --type java -A 2 -B 2Repository: PSMRI/AMRIT-DB
Length of output: 40
🏁 Script executed:
# Search for sterilisation/sterilization variants in Java
rg -i 'sterili[sz]ation|sterili[sz]ation_date' --type java -A 2 -B 2Repository: PSMRI/AMRIT-DB
Length of output: 40
🏁 Script executed:
# Check migration files for existing column definitions
rg -i 'steril' . --include='*.sql' -A 1 -B 1Repository: PSMRI/AMRIT-DB
Length of output: 499
🏁 Script executed:
# Look for PNCVisit or similar entity definitions
fd -e java -type f | xargs grep -l 'PNCVisit\|PnC\|pnc_visit' | head -20Repository: PSMRI/AMRIT-DB
Length of output: 228
🏁 Script executed:
# First, check if this is a database-only repository
ls -la | head -20Repository: PSMRI/AMRIT-DB
Length of output: 879
🏁 Script executed:
# Search SQL files for sterilisation/steilisation variants
rg -i 'steril' -t sqlRepository: PSMRI/AMRIT-DB
Length of output: 383
🏁 Script executed:
# Look at the context around the V58 migration file
cat -n src/main/resources/db/migration/dbiemr/V58__missing_migration.sqlRepository: PSMRI/AMRIT-DB
Length of output: 6252
🏁 Script executed:
# Check previous migrations for any sterilisation column references
rg -i 'steril' src/main/resources/db/migration/dbiemr/ -t sql -B 1 -A 1Repository: PSMRI/AMRIT-DB
Length of output: 770
steilisation_date is a confirmed typo — fix before this migration runs.
steilisation is a transposition error (ei ↔ ie). The correct spelling is sterilisation_date, which is consistent with the date_of_sterilisation column added in V48 for the ANC visit table. This permanent schema defect will break any application entity mapping that expects the correct spelling.
🐛 Proposed fix
- SET `@columnname` = 'steilisation_date';
+ SET `@columnname` = 'sterilisation_date';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SET @columnname = 'steilisation_date'; | |
| SET @preparedStatement = (SELECT IF( | |
| (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = @dbname AND TABLE_NAME = @tablename AND COLUMN_NAME = @columnname) > 0, | |
| 'SELECT 1', | |
| CONCAT('ALTER TABLE ', @dbname, '.', @tablename, ' ADD COLUMN ', @columnname, ' TIMESTAMP NULL;') | |
| )); | |
| PREPARE stmt FROM @preparedStatement; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; | |
| SET `@columnname` = 'sterilisation_date'; | |
| SET `@preparedStatement` = (SELECT IF( | |
| (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = `@dbname` AND TABLE_NAME = `@tablename` AND COLUMN_NAME = `@columnname`) > 0, | |
| 'SELECT 1', | |
| CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, ' TIMESTAMP NULL;') | |
| )); | |
| PREPARE stmt FROM `@preparedStatement`; | |
| EXECUTE stmt; | |
| DEALLOCATE PREPARE stmt; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V58__missing_migration.sql` around
lines 61 - 69, The migration uses the misspelled column name 'steilisation_date'
via the variable `@columnname` and prepared statement (preparedStatement/stmt);
update the migration to use the correct column name 'sterilisation_date'
everywhere (set `@columnname`, any CONCAT for ALTER TABLE, and any existence
checks) so the migration adds/checks sterilisation_date (matching V48's
date_of_sterilisation naming), and run a quick search to ensure no downstream
code or other migrations reference the old misspelling before committing.
| SET @columnname = 'date'; | ||
| SET @preparedStatement = (SELECT IF( | ||
| (SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = @dbname AND TABLE_NAME = @tablename AND COLUMN_NAME = @columnname) > 0, | ||
| 'SELECT 1', | ||
| CONCAT('ALTER TABLE ', @dbname, '.', @tablename, ' ADD COLUMN ', @columnname, ' DATE DEFAULT NULL;') | ||
| )); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MySQL 8.0 DATE reserved keyword backtick quoting column name syntax error
💡 Result:
In MySQL 8.0, DATE / date can conflict with SQL parsing (it’s a keyword), so if you used it as a column name you must treat it as an identifier and quote it everywhere with backticks (not single quotes) [1].
Examples:
CREATE TABLE t (
`date` DATE NOT NULL
);
SELECT `date` FROM t;
INSERT INTO t (`date`) VALUES ('2026-02-18');
ALTER TABLE t MODIFY COLUMN `date` DATE NOT NULL;Common gotcha: if your “syntax error” is around a DEFAULT, note that as of MySQL 8.0.13 default values may be expressions, but expression defaults must generally be wrapped in parentheses [2][3]:
CREATE TABLE t (
`date` DATE NOT NULL DEFAULT (CURRENT_DATE)
);Best practice is still to rename the column (e.g., created_date, event_date) to avoid having to quote it in every query [1].
Sources: [1] MySQL 8.0 manual: keywords/reserved words and quoting identifiers, [2] MySQL 8.0 manual: default value rules (notably changes as of 8.0.13), [3] MySQL 8.0 example using DEFAULT (CURRENT_DATE) for a DATE column.
🏁 Script executed:
cat -n src/main/resources/db/migration/dbiemr/V59__sammelan_record.sqlRepository: PSMRI/AMRIT-DB
Length of output: 1250
date is a reserved MySQL keyword — the generated ALTER TABLE will fail with a syntax error unless the identifier is quoted
DATE is a reserved keyword in MySQL 8.0. When used as a column name, it must be enclosed in backticks to distinguish it from the keyword itself. The CONCAT on line 24 currently builds:
ALTER TABLE db_iemr.uwin_session_record ADD COLUMN date DATE DEFAULT NULL;
The unquoted date identifier will cause MySQL to reject the statement with Error 1064: You have an error in your SQL syntax when the migration executes.
Fix 1 (preferred): rename the column to something unambiguous (e.g., session_date — consistent with the existing pattern in sammelan_record — or event_date).
Fix 2 (if the name must stay): backtick-quote the identifier in the CONCAT:
🛠️ Proposed fix (backtick quoting)
- CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN ', `@columnname`, ' DATE DEFAULT NULL;')
+ CONCAT('ALTER TABLE `', `@dbname`, '`.`', `@tablename`, '` ADD COLUMN `', `@columnname`, '` DATE DEFAULT NULL;')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/db/migration/dbiemr/V59__sammelan_record.sql` around lines
20 - 25, The migration builds an ALTER TABLE to add a column named 'date' which
is a reserved MySQL keyword; change the schema to use a non-reserved name (e.g.,
set `@columnname` = 'session_date' to match sammelan_record pattern) and update
any downstream references, or if the column name must remain 'date' modify the
CONCAT that builds `@preparedStatement` to wrap the identifier in backticks (e.g.,
use CONCAT('ALTER TABLE ', `@dbname`, '.', `@tablename`, ' ADD COLUMN `',
`@columnname`, '` DATE DEFAULT NULL;')) and also ensure `@dbname` and `@tablename` are
properly quoted if needed; update usage of `@columnname` and `@preparedStatement`
accordingly.
8 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Bug Fixes
Chores
New Features