age
age copied to clipboard
Simplify exec_cypher_set
- Initializing slot to NULL right at the declaration, which ensures that it has a valid value regardless of the execution path through the function.
- Removing the multiple instances of
estate->es_result_relation_info = saved_resultRelInfo;
and completely eliminatingsaved_resultRelInfo
. It seems this was not affecting any functionality in the code as the removal ofes_result_relation_info restoration
did not affect regression tests. - Refactoring the if and else conditions to avoid redundant code, which simplifies the function and makes it easier to read, with a clearer control flow.
Regression tests:
/usr/local/pgsql/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress --inputdir=./ --bindir='/usr/local/pgsql/bin' --load-extension=age --inputdir=.//regress --outputdir=.//regress --temp-instance=.//regress/instance --port=61958 --encoding=UTF-8 --dbname=contrib_regression scan graphid agtype catalog cypher expr cypher_create cypher_match cypher_unwind cypher_set cypher_remove cypher_delete cypher_with cypher_vle cypher_union cypher_call cypher_merge age_global_graph age_load index analyze graph_generation name_validation drop
============== creating temporary instance ==============
============== initializing database system ==============
============== starting postmaster ==============
running on port 61958 with PID 23622
============== creating database "contrib_regression" ==============
CREATE DATABASE
ALTER DATABASE
============== installing age ==============
CREATE EXTENSION
============== running regression test queries ==============
test scan ... ok 250 ms
test graphid ... ok 16 ms
test agtype ... ok 168 ms
test catalog ... ok 156 ms
test cypher ... ok 28 ms
test expr ... ok 641 ms
test cypher_create ... ok 144 ms
test cypher_match ... ok 563 ms
test cypher_unwind ... ok 61 ms
test cypher_set ... ok 150 ms
test cypher_remove ... ok 99 ms
test cypher_delete ... ok 131 ms
test cypher_with ... ok 100 ms
test cypher_vle ... ok 1059 ms
test cypher_union ... ok 37 ms
test cypher_call ... ok 49 ms
test cypher_merge ... ok 221 ms
test age_global_graph ... ok 187 ms
test age_load ... ok 1556 ms
test index ... ok 100 ms
test analyze ... ok 28 ms
test graph_generation ... ok 99 ms
test name_validation ... ok 146 ms
test drop ... ok 223 ms
============== shutting down postmaster ==============
============== removing temporary instance ==============
======================
All 24 tests passed.
======================
@markgomer Could you rebase/update this against the master branch? :)
@markgomer @jrgemignani any follow-up on this?
Sorry for the delay, I've been out of it for a while. I have tested it in my own fork and it should be fine now. Please let me know
@dehowef @rafsun42 @MuhammadTahaNaveed @Zainab-Saad Can I get your thoughts on this?
@markgomer I am of the opinion that these specific changes are unnecessary and don't really make the code any more clear. That is not to say that I don't appreciate the effort. Additionally, the original author had poor coverage with regression tests so, I would be cautious about using them as a metric here.
@jrgemignani I agree to be honest, so I am closing this PR. Back in the day I thought it would be ok if the regress tests passed, and they got a tiny boost in performance in my machine. But I see your point about the changes. They don’t really add much and may break something unknown to me.