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

Deparse removes parentheses around CURRENT_TIMESTAMP AT TIME ZONE 'UTC' in table creation DEFAULT clause #126

Open
tylergannon opened this issue Dec 15, 2024 · 1 comment

Comments

@tylergannon
Copy link

tylergannon commented Dec 15, 2024

Affected library: github.com/pganalyze/pg_query_go/v6 (and likely the underlying C library)

Description:

When parsing a CREATE TABLE statement that specifies a DEFAULT value with CURRENT_TIMESTAMP at a time zone, the original query contains parentheses around the expression.

The result of pg_query.Deparse() on such a statement is syntactically invalid.

I assume this issue is actually in regards to the C library backing the Go library, but I'm reporting it here because I'm more capable of making good test code in Go.

Steps to Reproduce:

  1. Call pg_query.Parse() with a CREATE TABLE statement containing a DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC').
  2. Deparse the resulting parse tree using pg_query.Deparse().
  3. Notice that the returned CREATE TABLE statement no longer includes the parentheses around the time zone expression in the DEFAULT clause, producing invalid SQL syntax.

Expected Behavior:

Parse and Deparse should be commutative, meaning that the results of Deparse should be, in turn, Parseable, e.g.:

CREATE TABLE my_table (created_at TIMESTAMPTZ NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC'))

Actual Behavior:

Deparsing removes the parentheses, producing:

CREATE TABLE my_table (created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP AT TIME ZONE 'UTC')

If we run this through pg_query.Parse, we get: syntax error at or near "AT"

Environment:
Go 1.23.3
pg_query_go version: v5, v6 both tested
platform: Darwin (15.2)

Test code

package main

import (
	"fmt"
	"log"

	"github.com/pganalyze/pg_query_go/v6"
)

func main() {
	// Original query with parentheses around the DEFAULT expression
	originalQuery := `CREATE TABLE my_table (created_at TIMESTAMPTZ NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC'))`

	// Parse the original query
	parseResult, err := pg_query.Parse(originalQuery)
	if err != nil {
		log.Fatalf("Unexpected parse error: %v", err)
	}
	if parseResult == nil {
		log.Fatalf("Parse result is nil")
	}

	// Deparse the resulting parse tree
	deparsed, err := pg_query.Deparse(parseResult)
	if err != nil {
		log.Fatalf("Unexpected deparse error: %v", err)
	}

	fmt.Println("Original query:")
	fmt.Println(originalQuery)
	fmt.Println("Deparsed query:")
	fmt.Println(deparsed)
}
@tylergannon
Copy link
Author

My first intuition was that there would be a broader issue with complex expressions in DEFAULT value declarations. Indeed Deparse() does remove the parens from complex expressions, but in the additional cases I tried, the result is still valid SQL.

In the below test, it's only the CURRENT_TIMESTAMP AT TIME ZONE 'UTC' example that fails.

Note that other similar cases do fail, e.g. DEFAULT (now() AT TIME ZONE 'UTC')

package main

import (
	"fmt"
	"strings"
	"testing"

	"github.com/pganalyze/pg_query_go/v6"
)

func TestDeparseComplexDefaults(t *testing.T) {
	// We'll test a range of complex default expressions often found in Postgres schemas.
	// For each test, we:
	// 1) Parse the given CREATE TABLE statement.
	// 2) Deparse the resulting parse tree back to SQL.
	// 3) Parse the deparsed SQL again to ensure it's syntactically valid.
	//
	// We aren't strictly asserting that the original and deparsed SQL are identical strings,
	// because pg_query may adjust formatting or other minor details. However, we do check
	// that the resulting SQL is at least syntactically valid and doesn't lose critical
	// parentheses or operators. If the re-parsed result fails, that's a problem.
	//
	// These tests are intended to help identify whether complex DEFAULT expressions
	// are handled correctly, or if certain constructs lose parentheses or cause syntax errors.

	tests := []struct {
		name  string
		query string
	}{
		{
			name: "TimestampWithTimeZoneParentheses",
			query: `CREATE TABLE my_table (
				created_at TIMESTAMPTZ NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC')
			)`,
		},
		{
			name: "ComplexArithmeticExpression",
			query: `CREATE TABLE my_table (
				next_day TIMESTAMPTZ NOT NULL DEFAULT (CURRENT_TIMESTAMP + INTERVAL '1 day')
			)`,
		},
		{
			name: "TypeCastInDefault",
			query: `CREATE TABLE my_table (
				created_at DATE NOT NULL DEFAULT (CURRENT_TIMESTAMP::date)
			)`,
		},
		{
			name: "FunctionCall",
			query: `CREATE TABLE my_table (
				checksum TEXT NOT NULL DEFAULT (md5('test_string'))
			)`,
		},
		{
			name: "SequenceNextVal",
			query: `CREATE TABLE my_table (
				id BIGINT NOT NULL DEFAULT (nextval('my_seq'))
			)`,
		},
		{
			name: "CaseExpression",
			query: `CREATE TABLE my_table (
				val TEXT NOT NULL DEFAULT (
					CASE WHEN EXTRACT(HOUR FROM CURRENT_TIMESTAMP) < 12 THEN 'morning' ELSE 'afternoon' END
				)
			)`,
		},
		{
			name: "CoalesceNullif",
			query: `CREATE TABLE my_table (
				coalesced_val TEXT NOT NULL DEFAULT (
					COALESCE(NULLIF('',''), 'fallback')
				)
			)`,
		},
		{
			name: "NestedExpressions",
			query: `CREATE TABLE my_table (
				complex_val INT NOT NULL DEFAULT (
					(GREATEST(1, LEAST(2,3))) + (ABS(-5))
				)
			)`,
		},
		{
			name: "MultipleColumnsMultipleComplexDefaults",
			query: `CREATE TABLE my_table (
				col1 TIMESTAMPTZ NOT NULL DEFAULT (CURRENT_TIMESTAMP + INTERVAL '2 hours'),
				col2 TEXT NOT NULL DEFAULT (LOWER(md5('SomeText'))),
				col3 INT NOT NULL DEFAULT (2 * (3 + 4))
			)`,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			// Parse the original query
			parseResult, err := pg_query.Parse(tt.query)
			if err != nil {
				t.Fatalf("Failed to parse original query: %v\nQuery: %s", err, tt.query)
			}

			// Deparse the parse tree back to SQL
			deparsed, err := pg_query.Deparse(parseResult)
			if err != nil {
				t.Fatalf("Failed to deparse: %v\nOriginal Query: %s", err, tt.query)
			}

			fmt.Println(deparsed)

			// Ensure the deparsed SQL is syntactically valid by parsing it again
			// If parsing fails here, then the deparsed SQL is invalid.
			_, err = pg_query.Parse(deparsed)
			if err != nil {
				t.Errorf("Deparsed query is not valid SQL: %v\nOriginal: %s\nDeparsed: %s", err, tt.query, deparsed)
			}

			// Optional: Check if parentheses and critical parts are still present.
			// This is more heuristic and may need to be adapted as pg_query evolves.
			// For now, just a sanity check that we're not obviously losing parentheses
			// that appear in the original.
			if strings.Contains(tt.query, "(") && !strings.Contains(deparsed, "(") {
				t.Logf("Warning: Deparsed query lost parentheses present in the original. This may or may not be a bug.\nOriginal: %s\nDeparsed: %s", tt.query, deparsed)
			}
		})
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant