Skip to content

fix: filter columns by java field name for camel case field names - #1130

Open
CodeMan-cmd wants to merge 1 commit into
apache:mainfrom
CodeMan-cmd:fix-include-exclude-camel-case-field-name
Open

CodeMan-cmd wants to merge 1 commit into
apache:mainfrom
CodeMan-cmd:fix-include-exclude-camel-case-field-name

Conversation

@CodeMan-cmd

Copy link
Copy Markdown

The column filter compared the name kept by the cglib bean map of a field with the field names configured through includeColumnFieldNames/excludeColumnFieldNames. For a field whose name starts with a lower case letter followed by an upper case letter, for example xRealIp or pName, the two names differ, so includeColumnFieldNames dropped the column and excludeColumnFieldNames kept it. The reorder by include list also assigned a position to an unknown column name, which shifted the following columns.

Purpose of the pull request

What's changed?

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

The column filter compared the name kept by the cglib bean map of a field with the field names configured through includeColumnFieldNames/excludeColumnFieldNames. For a field whose name starts with a lower case letter followed by an upper case letter, for example xRealIp or pName, the two names differ, so includeColumnFieldNames dropped the column and excludeColumnFieldNames kept it. The reorder by include list also assigned a position to an unknown column name, which shifted the following columns.

@skytin1004 skytin1004 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix. Using the Java field name is the right approach because include/exclude names are supplied through the public API, and skipping unknown names avoids gaps in the requested column order. I reproduced the issue with the previous implementation and verified the fix. All relevant tests passed locally with Java 11. LGTM.

This branch has not been deployed

No deployments
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