[
https://issues.apache.org/jira/browse/CALCITE-4156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17172523#comment-17172523
]
Julian Hyde commented on CALCITE-4156:
--------------------------------------
I don't agree with this change.
That assert is perfect as an assert. It gives hints to the developer.
I assume that this problem showed up at development time. If so, if you had
been running with asserts enabled, you would have gotten the message. The real
take-away is that you, as a developer, should turn on asserts. Don't blame the
code.
> ReflectiveRelMetadataProvider constructor should throw an exception (instead
> of assertion) when called with an empty map
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-4156
> URL: https://issues.apache.org/jira/browse/CALCITE-4156
> Project: Calcite
> Issue Type: Task
> Components: core
> Reporter: Ruben Q L
> Assignee: Ruben Q L
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> ReflectiveRelMetadataProvider's constructor verifies that it is not created
> with an empty map, using an assertion. However, this is not the most reliable
> way of verifying this situation, since assertions can be deactivated. In such
> scenario, we could silently end up having an invalid
> ReflectiveRelMetadataProvider, with no actual methods attached.
> Also, since the map is private and has no getter, there is no way for a
> caller module to verify this situation on its side.
> For this reason, it is proposed a minor change: replace the assertion with an
> IllegalArgumentException, which will work in 100% of the cases and will
> always prevent constructing an invalid ReflectiveRelMetadataProvider.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
