search

python-postgres / be

pg-python: Execute Python 3 code from PostgreSQL functions.
Other
5 stars 2 forks source link

SRF_SHOULD_MATERIALIZE clarification #36

Closed EvanCarroll closed 2 years ago

EvanCarroll commented 2 years ago

You have code like this, if I read it right

#define SRF_SHOULD_MATERIALIZE(FCINFO) (
  (
    ((ReturnSetInfo *) FCINFO->resultinfo)->allowedModes & SFRM_Materialize
  )
  && (
    !(
      ((ReturnSetInfo *) FCINFO->resultinfo)->allowedModes & SFRM_ValuePerCall
    )
    || ((
      ((ReturnSetInfo *) FCINFO->resultinfo)->allowedModes & SFRM_Materialize_Preferred
    ))
  )
)

This raises the question to me what happens if SFRM_Materialize is set but neither SFRM_Materialize_Preferred nor SFRM_ValuePerCall?

EvanCarroll commented 2 years ago

Misread it. Glad I reformatted it.

jwp commented 2 years ago

A formatting issue for sure. A few concise access macros to make the logic more readable would have been helpful. "_materialize(modes) && (!_vpc(modes) || _preferred(modes))"

EvanCarroll commented 2 years ago

I was actually reading it wrong. But it doesn't actually matter because SFRM_Materialize and SFRM_ValuePerCall is ALWAYS true. So this reduces to,

#define SRF_SHOULD_MATERIALIZE(FCINFO) (
  true
  && (
    !(true)
    || ((
      ((ReturnSetInfo *) FCINFO->resultinfo)->allowedModes & SFRM_Materialize_Preferred
    ))
  )
)

In other words

true && ( false || ((ReturnSetInfo *) FCINFO->resultinfo)->allowedModes & SFRM_Materialize_Preferred )

See https://github.com/EvanCarroll/pg-dump-fcinfo if you want to test it.

jwp commented 2 years ago

The code may have been written defensively... I'm not sure if that is actually coherent here(pointlessly defensive ;). It's getting close to ten years since I've looked at it. It definitely begs for a (better) comment or two in addition to better formatting.

https://github.com/python-postgres/be/blob/880a5336498ec10ad169034f4badb635e2674065/src/pl.c#L1652-L1658 https://github.com/python-postgres/be/blob/880a5336498ec10ad169034f4badb635e2674065/src/include/pypg/postgres.h#L129-L147