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

Fix index-creating queries in ddl script #4758

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yoseplee
Copy link

@yoseplee yoseplee commented Feb 5, 2025

Motivation

  • Me and my colleague are recently trying MongoDB as a job repository.
  • According to the document I was able to access the example DDL scripts.
  • I run the script without reading it carefully, and it failed, and fixed it.
    • Usage of createIndex() was wrong.
    • Name of the index duplicated which made error as well.

What I did

  • Fixed createIndex() query.
    • According to the syntax, it should have looked like db.collection.createIndex(<keys>, <options>, <commitQuorum>) but was likedb.collection.createIncex(<name>, <keys>, <commitQuorum>).
  • Renamed duplicated index names of BATCH_JOB_EXECUTION collection.

Note

  • Unlike the script, I read through the document and found suggestions about indexing metadata tables.
  • Please let me know the duplicated name in the script is intended.
  • In addition, improving integration test case that is introduced in What's new in Spring Batch 5.2 might be a good idea.

@yoseplee yoseplee force-pushed the fix-mongo-ddl-script branch 3 times, most recently from 25cf1e7 to 165d355 Compare February 5, 2025 12:04
@yoseplee yoseplee force-pushed the fix-mongo-ddl-script branch from 165d355 to 1586259 Compare February 5, 2025 12:34
@yoseplee yoseplee changed the title Fix duplicated index names in ddl script Fix index-creating queries in ddl script Feb 5, 2025
@fmbenhassine
Copy link
Contributor

Hi, thank you for the PR!

Regarding the syntax issue, which version of mongo db are you using? Is it the same as the one in the integration test?

Please let me know the duplicated name in the script is intended.

Indeed, I think the following indexes have duplicate names:

db.getCollection("BATCH_JOB_EXECUTION").createIndex("job_instance_idx", {"jobInstanceId": 1}, {});
db.getCollection("BATCH_JOB_EXECUTION").createIndex("job_instance_idx", {"jobInstanceId": 1, "status": 1}, {});

The second one should be something like job_instance_status_idx. Please update the PR accordingly.

In addition, improving integration test case that is introduced in What's new in Spring Batch 5.2 might be a good idea.

yes probably that was missed, the test should include the index creation statements exactly in the same way as in the script schema-mongodb.js. Please add that to the PR and it should be good to merge.

Many thanks upfront!

@fmbenhassine fmbenhassine added pr-for: bug status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter in: core labels Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core pr-for: bug status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants