I noticed that some places in our code test "SvOK(sv) && SvROK(sv)"
while others check just SvROK(sv). On investigation, it's clear
that SvROK implies SvOK so the first part of these tests is
pointless. I find in the Perl sources (sv.h):
#define SVf_IOK 0x00000100 /* has valid public integer value */
#define SVf_NOK 0x00000200 /* has valid public numeric value */
#define SVf_POK 0x00000400 /* has valid public pointer value */
#define SVf_ROK 0x00000800 /* has a valid reference pointer */
...
#define SVf_OK (SVf_IOK|SVf_NOK|SVf_POK|SVf_ROK| \
SVp_IOK|SVp_NOK|SVp_POK|SVpgv_GP)
...
#define SvOK(sv) (SvFLAGS(sv) & SVf_OK)
...
#define SvROK(sv) (SvFLAGS(sv) & SVf_ROK)
SvOK() is evidently intended to encode "has a defined value of any
type" while SvROK() specifically means "has a reference value".
So I think we can make our code more idiomatic and (doubtless
not measurably) faster as attached.
regards, tom lane
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index ace39cd072b..62e216978f4 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -1095,7 +1095,7 @@ get_perl_array_ref(SV *sv)
{
dTHX;
- if (sv && SvOK(sv) && SvROK(sv))
+ if (sv && SvROK(sv))
{
if (SvTYPE(SvRV(sv)) == SVt_PVAV)
return sv;
@@ -1107,8 +1107,7 @@ get_perl_array_ref(SV *sv)
if (sav && *sav)
{
plperl_materialize_sv(*sav);
- if (SvOK(*sav) && SvROK(*sav) &&
- SvTYPE(SvRV(*sav)) == SVt_PVAV)
+ if (SvROK(*sav) && SvTYPE(SvRV(*sav)) == SVt_PVAV)
return *sav;
}
@@ -1767,7 +1766,7 @@ plperl_modify_tuple(HV *hvTD, TriggerData *tdata, HeapTuple otup)
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg("$_TD->{new} does not exist")));
plperl_materialize_sv(*svp);
- if (!SvOK(*svp) || !SvROK(*svp) || SvTYPE(SvRV(*svp)) != SVt_PVHV)
+ if (!(*svp) || !SvROK(*svp) || SvTYPE(SvRV(*svp)) != SVt_PVHV)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("$_TD->{new} is not a hash reference")));
@@ -3357,7 +3356,7 @@ plperl_return_next_internal(SV *sv)
HeapTuple tuple;
plperl_materialize_sv(sv);
- if (!(SvOK(sv) && SvROK(sv) && SvTYPE(SvRV(sv)) == SVt_PVHV))
+ if (!(SvROK(sv) && SvTYPE(SvRV(sv)) == SVt_PVHV))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("SETOF-composite-returning PL/Perl function "
