Skip to content

Auto-populate Int/Float Fields#257

Open
JCRPaquin wants to merge 1 commit into
mainfrom
jcrpaquin/improvement/auto-int-fields
Open

Auto-populate Int/Float Fields#257
JCRPaquin wants to merge 1 commit into
mainfrom
jcrpaquin/improvement/auto-int-fields

Conversation

@JCRPaquin
Copy link
Copy Markdown
Member

What?

Uses schema information to determine which fields should be parsed as integers/floats.

Why?

We currently use aqp.intFields and aqp.floatFields to populate the list of fields to parse as each type. This leads to oversights where some fields don't get parsed properly because they were not added to the appropriate list. This change prevents that class of mistakes.

@JCRPaquin JCRPaquin requested a review from kelockhart April 21, 2026 19:59

req.getSchema().getFields().forEach((fieldName, schemaField) -> {
switch (schemaField.getType().getNumberType()) {
case FLOAT, DOUBLE -> ncm.put(fieldName, new PointsConfig(new MaxNumberFormat(Float.MAX_VALUE), Float.class));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem safe to map DOUBLE into Float - is there a good reason not to make another case statement? for DOUBLE?

req.getSchema().getFields().forEach((fieldName, schemaField) -> {
switch (schemaField.getType().getNumberType()) {
case FLOAT, DOUBLE -> ncm.put(fieldName, new PointsConfig(new MaxNumberFormat(Float.MAX_VALUE), Float.class));
case INTEGER, LONG -> ncm.put(fieldName, new PointsConfig(new MaxNumberFormat(Integer.MAX_VALUE), Integer.class));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment, but for LONG - why not have another case statement to handle LONG?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants