Hello Jeevan,
> I did the review of your patch and here are my views on your patch.
Thanks for this detailed review and debugging!
> Documentation: [...] it be a good idea to have a table of operators
> similar to that of functions. We need not have several columns here like
> description, example etc., but a short table just categorizing the
> operators would be sufficient.
Ok, done.
> Further testing and review:
> ===========================
> 1. Postgres treats '^' as exponentiation rather than XOR, and '#' as XOR.
> Personally, I think it can cause confusion, so it will be better if we can stick
> to the behavior of Postgres mathematical operators.
Ok. I agree to avoid '^'.
> 2. I could not see any tests for bitwise operators in the functions.sql
> file that you have attached.
Indeed. Included in attached version.
> 3. Precedence: [...]
Hmm. I got them all wrong, shame on me! I've followed C rules in the
updated version.
> 5. Sorry, I was not able to understand the "should exist" comment in following
> snippet.
>
> +"xor" { return XOR_OP; } /* should exist */
> +"^^" { return XOR_OP; } /* should exist */
There is no "logical exclusive or" operator in C nor in SQL. I do not see
why not, so I put one...
> 7. You may want to reword following comment: [...] there -> here
Ok, fixed twice.
> 8. [...] if (coerceToBool(&vargs[0])) *retval = vargs[1]; else *retval = vargs[2];
Ok.
Attached is an updated patch & test script.
--
Fabien.
