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

Inefficient pretty printing causes stack overflow #1400

Open
RCHowell opened this issue Mar 25, 2024 · 0 comments
Open

Inefficient pretty printing causes stack overflow #1400

RCHowell opened this issue Mar 25, 2024 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@RCHowell
Copy link
Contributor

RCHowell commented Mar 25, 2024

Description

The pretty-printer is loosely based upon Wadler Prettier-Printer. In an attempt a faithful modeling of the data structures, then end result is an inefficient printer. We can greatly reduce the stack size and gain some speed with a reduced (but equivalent) model.

The primary bug we are facing now is that query expansions cause the pretty printer to stack overflow.

-- v1 as int
COALESCE(COALESCE(COALESCE(V1)))

-- expands to
CASE WHEN NOT (CASE WHEN NOT (CASE WHEN NOT (V1 IS NULL) THEN V1 ELSE CAST(NULL AS INT) END IS NULL) THEN CASE WHEN NOT (V1 IS NULL) THEN V1 ELSE CAST(NULL AS INT) END ELSE CAST(NULL AS INT) END IS NULL) THEN CASE WHEN NOT (CASE WHEN NOT (V1 IS NULL) THEN V1 ELSE CAST(NULL AS INT) END IS NULL) THEN CASE WHEN NOT (V1 IS NULL) THEN V1 ELSE CAST(NULL AS INT) END ELSE CAST(NULL AS INT) END ELSE CAST(NULL AS INT) END

The expansion follows from an SQL equivalence, but the duplication of V1 makes this balloon.

To Reproduce

Steps to reproduce the behavior:

Parse and pretty-print the expanded query.

Expected Behavior

  • < A clear and concise description of what you expected to happen. >

Additional Context

Alan has provided a detail write-up here as well.

partiql/partiql-scribe#26

Proposal

We can simplify the pretty printer and pick up some perf improvements via a simpler block tree like

sealed class Block {
    
    public var next: Block? = null
    
    object NL : Block
    
    class Text(val context: String): Block
    
    class Nest(
        val prefix: String?,
        val postfix: String?,
        val child: Block,
    ) : Block
}

For the curious, the current block tree is a giant linked list (some sublists) of tokens which is this way because that's how the prettier-printer paper algorithm is able to reflow a tree. That being said, the "pretty" part of the "pretty" printer is subjective and less important than having stack overflows are relatively small inputs.

For example, a current nest is like

link(link('{', link(child(....),  '}) 

And we see the link nodes approach the absurd, this is my bad 😄 so let's simplify this.

@RCHowell RCHowell added the bug Something isn't working label Mar 25, 2024
@RCHowell RCHowell changed the title Inefficient pretty printing Inefficient pretty printing causes stack overflow Mar 25, 2024
@RCHowell RCHowell self-assigned this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant