Re: Java Doc about RexProgramBuilderBase
Thank for your clarification! I’ll try to fix it shortly. Replied Message | From | Julian Hyde | | Date | 07/8/2023 02:16 | | To | | | Subject | Re: Java Doc about RexProgramBuilderBase | People refactoring code should remember that their IDE can move and rename fields but is not so good at changing documentation. (Maybe in a couple of years, with advances in generative AI?!) And when documentation and code don’t line up, people don’t trust either. (I’m as guilty of this as anyone.) On Jul 7, 2023, at 5:31 AM, Benchao Li wrote: This usually happens. When javadoc and the implementation diverges, the javadoc is mostly possible wrong and need to be improved to match the real behavior. For this specific case, I agree with you that the javadoc for `vParamNotNull` and `vDecimal(int arg)` are not correct, please fix them. (We can do this kind of trivial work without a Jira ticket) Zhe Hu 于2023年7月7日周五 14:45写道： Hi community. Recently, when I review CALCITE-5769( https://github.com/apache/calcite/pull/3296), I found something a little confusing. First, the java doc in RexProgramBuilderBase.vParamNotNull(), which meant to create non-nullable variable, but it’s returning description is “nullable varchar variable”. Second, we use vDecimal(int arg) to create nullable decimal variable, but the RelDataType we pass in is “nonNullableDecimal”, which I think should be “nullableDecimal”. So does the other vXxx() methods. I’m not sure if I understand right here. If it’s something we can improve, I’ll file a JIRA case to record and fix it. Best regards, Zhe Hu -- Best, Benchao Li
Re: Java Doc about RexProgramBuilderBase
Usually we need to improve the Java Doc or remove the misleading invalid code. Because the right code has the unit test to cover it. Yes, This is a bug and we should improve it. Feel free to submit PR for this. Julian Hyde 于2023年7月8日周六 02:16写道： > People refactoring code should remember that their IDE can move and rename > fields but is not so good at changing documentation. (Maybe in a couple of > years, with advances in generative AI?!) > > And when documentation and code don’t line up, people don’t trust either. > (I’m as guilty of this as anyone.) > > > On Jul 7, 2023, at 5:31 AM, Benchao Li wrote: > > > > This usually happens. When javadoc and the implementation diverges, the > > javadoc is mostly possible wrong and need to be improved to match the > real > > behavior. > > > > For this specific case, I agree with you that the javadoc for > > `vParamNotNull` and `vDecimal(int arg)` are not correct, please fix them. > > (We can do this kind of trivial work without a Jira ticket) > > > > Zhe Hu 于2023年7月7日周五 14:45写道： > > > >> Hi community. > >> Recently, when I review CALCITE-5769( > >> https://github.com/apache/calcite/pull/3296), I found something a > little > >> confusing. > >> > >> First, the java doc in RexProgramBuilderBase.vParamNotNull(), which > meant > >> to create non-nullable variable, but it’s returning description is > >> “nullable varchar variable”. > >> Second, we use vDecimal(int arg) to create nullable decimal variable, > but > >> the RelDataType we pass in is “nonNullableDecimal”, which I think > should be > >> “nullableDecimal”. So does the other vXxx() methods. > >> I’m not sure if I understand right here. If it’s something we can > improve, > >> I’ll file a JIRA case to record and fix it. > >> > >> > >> Best regards, > >> Zhe Hu > >> > >> > > > > -- > > > > Best, > > Benchao Li > >
Re: Java Doc about RexProgramBuilderBase
People refactoring code should remember that their IDE can move and rename fields but is not so good at changing documentation. (Maybe in a couple of years, with advances in generative AI?!) And when documentation and code don’t line up, people don’t trust either. (I’m as guilty of this as anyone.) > On Jul 7, 2023, at 5:31 AM, Benchao Li wrote: > > This usually happens. When javadoc and the implementation diverges, the > javadoc is mostly possible wrong and need to be improved to match the real > behavior. > > For this specific case, I agree with you that the javadoc for > `vParamNotNull` and `vDecimal(int arg)` are not correct, please fix them. > (We can do this kind of trivial work without a Jira ticket) > > Zhe Hu 于2023年7月7日周五 14:45写道： > >> Hi community. >> Recently, when I review CALCITE-5769( >> https://github.com/apache/calcite/pull/3296), I found something a little >> confusing. >> >> First, the java doc in RexProgramBuilderBase.vParamNotNull(), which meant >> to create non-nullable variable, but it’s returning description is >> “nullable varchar variable”. >> Second, we use vDecimal(int arg) to create nullable decimal variable, but >> the RelDataType we pass in is “nonNullableDecimal”, which I think should be >> “nullableDecimal”. So does the other vXxx() methods. >> I’m not sure if I understand right here. If it’s something we can improve, >> I’ll file a JIRA case to record and fix it. >> >> >> Best regards, >> Zhe Hu >> >> > > -- > > Best, > Benchao Li
Re: Java Doc about RexProgramBuilderBase
This usually happens. When javadoc and the implementation diverges, the javadoc is mostly possible wrong and need to be improved to match the real behavior. For this specific case, I agree with you that the javadoc for `vParamNotNull` and `vDecimal(int arg)` are not correct, please fix them. (We can do this kind of trivial work without a Jira ticket) Zhe Hu 于2023年7月7日周五 14:45写道： > Hi community. > Recently, when I review CALCITE-5769( > https://github.com/apache/calcite/pull/3296), I found something a little > confusing. > > First, the java doc in RexProgramBuilderBase.vParamNotNull(), which meant > to create non-nullable variable, but it’s returning description is > “nullable varchar variable”. > Second, we use vDecimal(int arg) to create nullable decimal variable, but > the RelDataType we pass in is “nonNullableDecimal”, which I think should be > “nullableDecimal”. So does the other vXxx() methods. > I’m not sure if I understand right here. If it’s something we can improve, > I’ll file a JIRA case to record and fix it. > > > Best regards, > Zhe Hu > > -- Best, Benchao Li
Java Doc about RexProgramBuilderBase
Hi community. Recently, when I review CALCITE-5769(https://github.com/apache/calcite/pull/3296), I found something a little confusing. First, the java doc in RexProgramBuilderBase.vParamNotNull(), which meant to create non-nullable variable, but it’s returning description is “nullable varchar variable”. Second, we use vDecimal(int arg) to create nullable decimal variable, but the RelDataType we pass in is “nonNullableDecimal”, which I think should be “nullableDecimal”. So does the other vXxx() methods. I’m not sure if I understand right here. If it’s something we can improve, I’ll file a JIRA case to record and fix it. Best regards, Zhe Hu
