Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

posthog.com

Post reply on HN
Print view View original

71–79 of 79 posts

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#71

Post by robbie-c »

cespare wrote: post #65

> We didn't write this parser by hand because, at least pre-AI-coding, parsers were extremely difficult to maintain. Writing one without AI would have taken months [...] > Instead, we use ANTLR, a state-of-the-art, open source parser generator. I don't agree with this (pre-AI-coding) take. Hand-rolled parsers are much easier to write well and maintain than people think. They also tend to be much faster and produce mu…

That's valid criticism, I kinda hand-waved the "months" part. I read everything I could about parsers while building this (I have a CS background but hadn't thought about parsers in a long time) and came across this blog post https://lakesail.com/blog/sql-parser-in-one-week/ which talked about building a toy parser in a week, so I scaled that up to months for a production one.

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#72

Post by bonzini »

robbie-c wrote: post #70
bonzini wrote: post #55

Earlier quoted context omitted.

I think you should clarify that (or whether) while you didn't look at the generated code, you are actually going to adjust it in the future. How did the two approaches compare in terms of code readability?

The previous parser is mostly a declarative grammar file, which is extremely readable. It codegens a C++ parser, which is hard to read. It depends which of those you count as the previous parser's source code! In the future, we'd make changes by modifying the ANTLR parser first, then using the same approach as in the blog post to get the new parser to parity. We have no plans to get rid of the C++ parser as an oracle…

Sorry, by two approaches I meant the two parsers generated by the LLM, recursive descent and graph based; not the ANTLR one.

Sort of unexpected that you're keeping the old one as an oracle—but a very good idea for anyone that writes such a complicated parser, indeed!

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#73

Post by spinachsalad »

> there’s a test for SELECT SELECT FROM FROM WHERE WHERE AND AND which is completely valid SQL

Is this even true? I tried it in SQLite and there's a syntax error after first SELECT. It would work when "SELECT", "FROM" etc. are quoted, but that's not the same thing.

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#74

Post by eudamoniac »

orsorna wrote: post #33
duke_of_vandals wrote: post #11

How long did this take?

About 1/1000 of the duration of their interview process where they gloat about wasting your time.

Surprised anyone gets an interview there. My resume matched the job requirements pretty much exactly with 11yoe and I got an auto rejection mail at 1am.

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#75

Post by charlieflowers »

spinachsalad wrote: post #73

> there’s a test for SELECT SELECT FROM FROM WHERE WHERE AND AND which is completely valid SQL Is this even true? I tried it in SQLite and there's a syntax error after first SELECT. It would work when "SELECT", "FROM" etc. are quoted, but that's not the same thing.

I interpreted it as referring to a table named "FROM" and fields named "SELECT", "WHERE" and "AND".

Re: I rewrote PostHog's SQL parser, 70x faster, while barely looking at the code

#76

Post by jamestexas »

This is super cool, and I am totally going to glean from how you handled testing some of this.

I have a tool I make as a data-plane to a graph engine, and it uses cap'n proto to help (And sqlite as a sort've IPC option). One of the biggest things I have is, I know I am not testing all of it to completion. I am not even really fuzzing, yet.

Thanks for sharing!

Post reply on HN