Hi hackers,

While reviewing CommitFest patch 6740, I noticed inconsistencies across the 
codebase regarding valid values for the EXPLAIN SERIALIZE option.

This is a documentation-only patch targeting the SERIALIZE option, 
which was originally added in CommitFest 2024-03 (patch #4852).

The parsing logic in `ParseExplainOptionList()` (explain_state.c) accepts 
the keyword `OFF` and treats it as an exact alias for `NONE`. This behavior 
is functional but currently undocumented.

I have identified four related inconsistencies:
1. The documented syntax only lists NONE | TEXT | BINARY
2. The `ExplainSerializeOption` enum in explain_state.h defines only three 
entries
3. psql tab completion only suggests NONE, TEXT and BINARY
4. Regression tests in explain.sql do not cover the OFF keyword

I initially thought about removing the OFF parsing logic to align all 
components. 
However, to preserve backward compatibility for existing queries and scripts, 
I chose not to make this code change.

This patch has been built, and the generated HTML documentation has been fully 
verified.

I confirmed the equivalence between OFF and NONE via the following tests:
```sql
explain (ANALYZE true, SERIALIZE off    ) select * from pg_catalog.pg_stat_lock;
explain (ANALYZE true, SERIALIZE none) select * from pg_catalog.pg_stat_lock;
```
Both statements execute identically with consistent runtime behavior.

To keep this patch minimal and focused, I leave the enum, tab completion and 
regression tests unchanged for now. This patch simply adds a note to the 
documentation stating that OFF is a valid alias for NONE. Any remaining 
inconsistencies can be addressed in separate follow-up patches later.


Reviews, comments and feedback are all welcome.

regards,
--
ZizhuanLiu (X-MAN) 
[email protected]

Attachment: v1-0001-DOCS-Mention-OFF-as-an-alias-for-EXPLAIN-SERIALIZ.patch
Description: Binary data

Attachment: sql-explain.html
Description: Binary data

Reply via email to