swjtu-zhanglei commented on PR #3837:
URL: https://github.com/apache/iceberg-python/pull/3837#issuecomment-5403759372
> I'm a little confused by this. Doesn't requests do this for us? We aren't
doing this anywhere else. Why is this the only place we're seeing this issue?
Thanks for the question. I traced through the full code path — requests
does not handle boolean serialization for us. Here's the detailed
evidence.
The full call chain with source references
Step 1: requests/models.py:473 — entry point
enc_params = self._encode_params(params)
Step 2: requests/models.py:107-134 — _encode_params
@staticmethod
def _encode_params(data):
...
elif hasattr(data, "__iter__"):
result = []
for k, vs in to_key_val_list(data):
if isinstance(vs, basestring) or not hasattr(vs, "__iter__"):
vs = [vs] # True → [True]
for v in vs:
if v is not None:
result.append(
(
k.encode("utf-8") if isinstance(k, str) else
k,
v.encode("utf-8") if isinstance(v, str) else
v, # ← bool is NOT str, kept as-is
)
)
return urlencode(result, doseq=True) # ← passes raw bool
to stdlib
Key point at line 129: v.encode("utf-8") if isinstance(v, str) else v — a
Python bool is not a str, so it is not encoded/converted here. The
raw True object is passed through to urlencode.
Step 3: urllib/parse.py:980 — stdlib urlencode(query, doseq=True)
With doseq=True, the code enters the branch at line 1029. For each (k, v)
pair:
# urllib/parse.py:1036-1048
if isinstance(v, bytes): # True is not bytes → skip
v = quote_via(v, safe)
l.append(k + '=' + v)
elif isinstance(v, str): # True is not str → skip
v = quote_via(v, safe, encoding, errors)
l.append(k + '=' + v)
else:
try:
itr = iter(v) # iter(True) → TypeError!
except TypeError:
v = quote_via(str(v), safe, encoding, errors) # ← str(True) =
"True" (capital T)
l.append(k + '=' + v)
The smoking gun is line ~1047: str(v) where v is Python True → produces
"True" (capitalized). There is no boolean-aware branch anywhere in this
path.
Why this is the only place we see the issue
A grep for params={ in pyiceberg/catalog/rest/__init__.py shows
drop_table is the only call site that passes a Python bool as a query param
value. All other params (snapshotId, branch, tag, etc.) are strings,
which hit the isinstance(v, str) branch and are handled correctly.
Other Iceberg implementations get this right
iceberg-go (catalog/rest/rest.go:1838,1864):
v.Set("purgeRequested", "false")
v.Set("purgeRequested", "true")
iceberg-rust (crates/catalog/rest/src/catalog.rs:738):
request_builder = request_builder.query(&[("purgeRequested", "true")]);
Both use explicit lowercase string literals — no implicit bool-to-string
conversion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
