Add duplicate column detection in EntityProvider

[okurashoichi]

PR Overview

This PR introduces improvements to Doma to handle scenarios where duplicate column names in query results could lead to unintended behavior during entity mapping. If this behavior is intentional in certain use cases, we are open to revisiting or withdrawing this change. The following enhancements have been implemented:

1. Duplicate Column Detection

   • Added logic to detect duplicate column names in query results.
   • Introduced a new exception, DuplicateColumnException, to provide detailed error information when duplicates are detected.

2. Related Code Changes

   • Enhanced the EntityProvider class with duplicate column detection logic.
   • Added a new error message to the Message enum.
   • Added unit tests to verify the behavior of the new logic.

---

Background

When mapping query results to entities, properties are mapped based on column names or the @Column annotation's name attribute. However, in cases where multiple tables are joined, and columns with the same name exist without specific annotations like @Table or @Column, the column from the joined table would take precedence, leading to unexpected behavior.

Investigation Findings

The following steps were taken to investigate the issue:

1. Setup
   Two tables, sample and sample_sub, were created, and a join query was executed as follows:

mysql> describe sample;
+-------+--------------+------+-----+---------+-------+
| Field | Type         | Null | Key | Default | Extra |
+-------+--------------+------+-----+---------+-------+
| id    | varchar(100) | NO   | PRI | NULL    |       |
| name  | varchar(100) | YES  |     | NULL    |       |
+-------+--------------+------+-----+---------+-------+
mysql> select * from sample;
+----+----------+
| id | name     |
+----+----------+
| 1  | sample01 |
+----+----------+

sample_sub

mysql> describe sample_sub;
+-----------+--------------+------+-----+---------+-------+
| Field     | Type         | Null | Key | Default | Extra |
+-----------+--------------+------+-----+---------+-------+
| id        | varchar(100) | NO   | PRI | NULL    |       |
| parent_id | varchar(100) | YES  | MUL | NULL    |       |
| sub_name  | varchar(100) | YES  |     | NULL    |       |
+-----------+--------------+------+-----+---------+-------+

mysql> select * from sample_sub;
+-----+-----------+------------+
| id  | parent_id | sub_name   |
+-----+-----------+------------+
| s01 | 1         | sub_name01 |
+-----+-----------+------------+

SQL templates File

   SELECT * FROM sample JOIN sample_sub ON sample.id = sample_sub.parent_id

Entity

@Entity(immutable = true)
@Table(name = "sample")
data class SampleJoin(
    val id: String,
    val name: String,
    @Column(name = "parent_id")
    val parentId: String,
    @Column(name = "sub_name")
    val subName: String,
)

dao

@Dao
@ConfigAutowireable
interface SampleJoinDao {
    @Select
    fun selectSampleJoin(): List<SampleJoin>
}

2. Execution Results
   When executed, the id column from the joined table (sample_sub) was mapped to the entity, as shown below:

SampleJoin(id=s01, name=sample01, parentId=1, subName=sub_name01)

[nakamura-to]

Thank you for your suggestion.
In general, duplicated columns are often considered a bug, so throwing an exception seems appropriate. However, considering that there might be users who rely on the current behavior, I think it would be better to make the DuplicateColumnException optional.
Specifically, I propose the following changes:

1. Introduce a new interface called DuplicateColumnHandler, with the current behavior set as the default implementation.
2. Add a new implementation class, ThrowingDuplicateColumnHandler, which throws a DuplicateColumnException.

If you agree with this approach, could you update your pull request accordingly?
Alternatively, I can merge the current pull request as it is and handle the changes on my side. Please let me know your thoughts.

[nakamura-to]

Please do not use wildcard (*) imports.

[okurashoichi]

f4d19de

[nakamura-to]

Please do not use wildcard (*) imports.

[okurashoichi]

1639737

[okurashoichi]

@nakamura-to
Thank you for your feedback.
I agree with your proposed approach to make the DuplicateColumnException optional.
I will update my pull request accordingly to add the DuplicateColumnHandler interface and its implementations.
Thank you for your guidance.

[nakamura-to]

Thank you for your fixes. I have added a few additional comments.

[nakamura-to]

Could you pass lowerCaseColumnName instead of columnName? The reason for this request is to align with the invocation of unknownColumnHandler.handle.

[okurashoichi]

b6806bb

[nakamura-to]

The test case name includes "DuplicateColumnNameException," but the test does not throw a DuplicateColumnNameException, which makes it confusing. Could you consider renaming it for clarity?

[okurashoichi]

79d2869

[nakamura-to]

Thanks so much for all your effort!