Skip to content

Conversation

@dreab8
Copy link
Member

@dreab8 dreab8 commented Feb 25, 2021

https://hibernate.atlassian.net/browse/HHH-9952

This is a rebased version of #1013.

I have also applied some changes to the original PR :

  • I changed the AbstractCollectionPersister#columnNames impl
  • I made a little change to the extractMutationTexts and moved it to the AbstractSelectExpression (not sure if this is the right place)

I'm not entirely sure about the impact of the change to AbstractEntityPersister#toColumns(String propertyName)

Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

The changes look ok to me mostly, but there is a test failure. The test org.hibernate.test.subselect.CompositeIdTypeBindingTest fails on databases that support row value constructors, like e.g. PostgreSQL.

Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

Changes look good now, but there are still failures for DBs that support the row-value constructor syntax like e.g. PostgreSQL in the following test: org.hibernate.test.subselect.CompositeIdTypeBindingTest

@dreab8
Copy link
Member Author

dreab8 commented Mar 1, 2021

Hi @beikov I'm working on that

@dreab8
Copy link
Member Author

dreab8 commented Mar 1, 2021

the issue should be fixed

@gavinking
Copy link
Member

This looks like it might impact HR, need to look more closely.

return scalarColumnIndex;
}

protected static String[] extractMutationTexts(Node operand, int count) {
Copy link
Contributor

Choose a reason for hiding this comment

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

}
String[] splits = StringHelper.split( ",", nodeText );
if ( count != splits.length ) {
throw new HibernateException( "SqlNode's text did not reference expected number of columns" );
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest creating a complete exception with more detail, like:

throw new HibernateException( String.format("sqlNode's text did not reference expected number of columns (expecting: %d, actual: %d)", count, splits.length));

Base automatically changed from master to main March 19, 2021 16:00
@Sanne
Copy link
Member

Sanne commented Oct 26, 2021

Was this abandoned?

@dreab8
Copy link
Member Author

dreab8 commented Dec 18, 2024

I created a test using annotation instead of the deprecated hbm mapping showing the issue has been resolved in recent Hibernate versions.

#9450

@dreab8 dreab8 closed this Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants