Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAXREC of exactly 2^31-1 produces a surprising error #155

Open
gpdf opened this issue Dec 14, 2023 · 1 comment
Open

MAXREC of exactly 2^31-1 produces a surprising error #155

gpdf opened this issue Dec 14, 2023 · 1 comment
Labels

Comments

@gpdf
Copy link

gpdf commented Dec 14, 2023

Motivated by a conversation on the IVOA dal mailing list, I've been looking at the handling of large values of MAXREC.

It appears that the CADC TAP code base rejects MAXREC values greater than the maximum signed 32-bit integer, i.e., 2,147,483,647 or 2^31 - 1, with a reasonable message, e.g.,

<INFO name="QUERY_STATUS" value="ERROR">Invalid MAXREC: 2147483648</INFO>

Values less than or equal to 2,147,483,646 appear to be accepted.

But if exactly 2,147,483,647 is provided, a curious error message appears:

<INFO name="QUERY_STATUS" value="ERROR">ERROR: LIMIT must not be negative</INFO>

Presumably this is because the service is using the well-known trick of supplying MAXREC+1 as a row limit to the underlying database, and then setting the overflow flag based on whether exactly MAXREC+1 rows come back. But in this case since MAXREC+1 can't be expressed in a signed 32-bit integer, it's not surprising that something weird might happen.

It's by no means an error that must be fixed -- feel free to simply close this report -- but perhaps just responding to MAXREC=2147483647 with the same "Invalid MAXREC" message would be a tiny improvement. Or you can leave it as an Easter egg for strange people like me to find.

@pdowler
Copy link
Member

pdowler commented Dec 20, 2023

Nice find! Indeed, maxrec + 1 is computed and put into the SQL LIMIT expression, but in java MAX_INT + 1 wraps around to -2147483648. The SQL gets executed and that's "ERROR: LIMIT ..." is from pg in this case.

In our services, the intent is to have an internal limit on async (because temp storage) and no limit on sync (because streaming)... there are a variety of ways to make this peculiar fail go away.

The purist in me wants to allow it and somehow predict that overflow won't happen (and not inject the LIMIT clause) but I know someone can write SQL that generates > 2^32 rows from a database that's smaller. It certainly doesn't seem worth much effort for such a niche usage...

@pdowler pdowler added the bug label Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants