From af966e662531ebfcf6d17c63b3cecfc21972c40b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Dec 2025 04:27:31 +0000 Subject: [PATCH 1/2] Add TestExplainVersionConsistency and integrate ClickHouse into regenerate-explain - Add TestExplainVersionConsistency test that validates all explain*.txt files have consistent ClickHouse version headers - Update cmd/regenerate-explain to download and manage ClickHouse v25.8.13.73-lts with HTTPS_PROXY support, idempotent start/stop, and version verification - Update parser tests to strip version headers when comparing EXPLAIN output - Remove scripts directory (functionality now in regenerate-explain command) --- cmd/regenerate-explain/main.go | 431 ++++++++++++++++++++++++++++++--- parser/parser_test.go | 98 ++++++++ scripts/clickhouse.sh | 230 ------------------ scripts/generate_explain.sh | 94 ------- 4 files changed, 494 insertions(+), 359 deletions(-) delete mode 100755 scripts/clickhouse.sh delete mode 100755 scripts/generate_explain.sh diff --git a/cmd/regenerate-explain/main.go b/cmd/regenerate-explain/main.go index b1f373dd58..b8d10f35a9 100644 --- a/cmd/regenerate-explain/main.go +++ b/cmd/regenerate-explain/main.go @@ -4,32 +4,74 @@ import ( "bytes" "flag" "fmt" + "io" + "net/http" + "net/url" "os" "os/exec" "path/filepath" + "strconv" "strings" + "syscall" + "time" +) + +const ( + // Required ClickHouse version for generating explain files + // Update this when regenerating all test expectations + requiredVersion = "25.8.1.3073" + requiredLTS = "25.8.13.73-lts" + + // Download URL for the static binary + downloadURL = "https://github.com/ClickHouse/ClickHouse/releases/download/v25.8.1.3073-lts/clickhouse-linux-amd64" +) + +var ( + clickhouseDir = ".clickhouse" + clickhouseBin = "clickhouse" + pidFile = filepath.Join(clickhouseDir, "clickhouse.pid") + configFile = filepath.Join(clickhouseDir, "config.xml") + serverLogFile = filepath.Join(clickhouseDir, "logs", "server.log") + serverErrFile = filepath.Join(clickhouseDir, "logs", "server.err.log") ) func main() { testName := flag.String("test", "", "Single test directory name to process (if empty, process all)") - clickhouseBin := flag.String("bin", "./clickhouse", "Path to ClickHouse binary") dryRun := flag.Bool("dry-run", false, "Print statements without executing") + serverOnly := flag.Bool("server", false, "Only ensure server is running, don't regenerate") + stopServer := flag.Bool("stop", false, "Stop the ClickHouse server") flag.Parse() - // Check if clickhouse binary exists - if !*dryRun { - if _, err := os.Stat(*clickhouseBin); os.IsNotExist(err) { - fmt.Fprintf(os.Stderr, "ClickHouse binary not found at %s\n", *clickhouseBin) - fmt.Fprintf(os.Stderr, "Run: ./scripts/clickhouse.sh download\n") + // Handle stop command + if *stopServer { + if err := stopClickHouse(); err != nil { + fmt.Fprintf(os.Stderr, "Error stopping ClickHouse: %v\n", err) os.Exit(1) } + fmt.Println("ClickHouse server stopped") + return + } + + // Ensure ClickHouse is running with correct version + if err := ensureClickHouse(); err != nil { + fmt.Fprintf(os.Stderr, "Error setting up ClickHouse: %v\n", err) + os.Exit(1) + } + + if *serverOnly { + fmt.Println("ClickHouse server is running with correct version") + return + } + + if *dryRun { + fmt.Println("Dry run mode - would process tests using clickhouse local") } testdataDir := "parser/testdata" if *testName != "" { // Process single test - if err := processTest(filepath.Join(testdataDir, *testName), *clickhouseBin, *dryRun); err != nil { + if err := processTest(filepath.Join(testdataDir, *testName), *dryRun); err != nil { fmt.Fprintf(os.Stderr, "Error processing %s: %v\n", *testName, err) os.Exit(1) } @@ -50,7 +92,7 @@ func main() { continue } testDir := filepath.Join(testdataDir, entry.Name()) - if err := processTest(testDir, *clickhouseBin, *dryRun); err != nil { + if err := processTest(testDir, *dryRun); err != nil { if strings.Contains(err.Error(), "no statements found") { skipped++ continue @@ -71,7 +113,334 @@ func main() { } } -func processTest(testDir, clickhouseBin string, dryRun bool) error { +// ensureClickHouse ensures ClickHouse is downloaded and running with the correct version +func ensureClickHouse() error { + // Check if already running with correct version + if version, err := getRunningVersion(); err == nil { + if version == requiredVersion { + fmt.Printf("ClickHouse %s is already running\n", version) + return nil + } + fmt.Printf("ClickHouse %s is running but need %s, restarting...\n", version, requiredVersion) + if err := stopClickHouse(); err != nil { + return fmt.Errorf("stopping existing ClickHouse: %w", err) + } + } + + // Download if binary doesn't exist or has wrong version + if err := ensureBinary(); err != nil { + return fmt.Errorf("ensuring binary: %w", err) + } + + // Start the server + if err := startClickHouse(); err != nil { + return fmt.Errorf("starting ClickHouse: %w", err) + } + + return nil +} + +// getRunningVersion checks if ClickHouse server is running and returns its version +func getRunningVersion() (string, error) { + // Check PID file + pidData, err := os.ReadFile(pidFile) + if err != nil { + return "", fmt.Errorf("no PID file: %w", err) + } + + pid, err := strconv.Atoi(strings.TrimSpace(string(pidData))) + if err != nil { + return "", fmt.Errorf("invalid PID: %w", err) + } + + // Check if process is running + process, err := os.FindProcess(pid) + if err != nil { + return "", fmt.Errorf("process not found: %w", err) + } + if err := process.Signal(syscall.Signal(0)); err != nil { + return "", fmt.Errorf("process not running: %w", err) + } + + // Query the server for version + cmd := exec.Command(clickhouseBin, "client", "--query", "SELECT version()") + var stdout bytes.Buffer + cmd.Stdout = &stdout + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("querying version: %w", err) + } + + return strings.TrimSpace(stdout.String()), nil +} + +// ensureBinary downloads ClickHouse if needed +func ensureBinary() error { + // Check if binary exists and has correct version + if _, err := os.Stat(clickhouseBin); err == nil { + version, err := getBinaryVersion() + if err == nil && version == requiredVersion { + fmt.Printf("ClickHouse binary %s already present\n", version) + return nil + } + if err == nil { + fmt.Printf("ClickHouse binary is %s but need %s, re-downloading...\n", version, requiredVersion) + } + } + + fmt.Printf("Downloading ClickHouse %s...\n", requiredLTS) + + // Create HTTP client with proxy support + client := &http.Client{ + Timeout: 10 * time.Minute, + } + + // Check for HTTPS_PROXY environment variable + if proxyURL := os.Getenv("HTTPS_PROXY"); proxyURL != "" { + proxy, err := url.Parse(proxyURL) + if err == nil { + client.Transport = &http.Transport{ + Proxy: http.ProxyURL(proxy), + } + fmt.Printf("Using HTTPS_PROXY: %s\n", proxyURL) + } + } + + req, err := http.NewRequest("GET", downloadURL, nil) + if err != nil { + return fmt.Errorf("creating request: %w", err) + } + + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("downloading: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("download failed with status %d", resp.StatusCode) + } + + // Write to temporary file first + tmpFile := clickhouseBin + ".tmp" + out, err := os.Create(tmpFile) + if err != nil { + return fmt.Errorf("creating temp file: %w", err) + } + + written, err := io.Copy(out, resp.Body) + out.Close() + if err != nil { + os.Remove(tmpFile) + return fmt.Errorf("writing binary: %w", err) + } + + fmt.Printf("Downloaded %d bytes\n", written) + + // Make executable and move to final location + if err := os.Chmod(tmpFile, 0755); err != nil { + os.Remove(tmpFile) + return fmt.Errorf("chmod: %w", err) + } + + if err := os.Rename(tmpFile, clickhouseBin); err != nil { + os.Remove(tmpFile) + return fmt.Errorf("rename: %w", err) + } + + // Verify version + version, err := getBinaryVersion() + if err != nil { + return fmt.Errorf("verifying binary: %w", err) + } + + if version != requiredVersion { + return fmt.Errorf("downloaded binary is version %s but expected %s", version, requiredVersion) + } + + fmt.Printf("ClickHouse %s installed successfully\n", version) + return nil +} + +// getBinaryVersion returns the version of the clickhouse binary +func getBinaryVersion() (string, error) { + cmd := exec.Command(clickhouseBin, "local", "--query", "SELECT version()") + var stdout bytes.Buffer + cmd.Stdout = &stdout + if err := cmd.Run(); err != nil { + return "", err + } + return strings.TrimSpace(stdout.String()), nil +} + +// startClickHouse starts the ClickHouse server +func startClickHouse() error { + // Create directories + dirs := []string{ + filepath.Join(clickhouseDir, "data"), + filepath.Join(clickhouseDir, "logs"), + filepath.Join(clickhouseDir, "tmp"), + filepath.Join(clickhouseDir, "user_files"), + filepath.Join(clickhouseDir, "format_schemas"), + } + for _, dir := range dirs { + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("creating directory %s: %w", dir, err) + } + } + + // Create config file + if err := writeConfig(); err != nil { + return fmt.Errorf("writing config: %w", err) + } + + fmt.Println("Starting ClickHouse server...") + + // Start the server as a daemon + cmd := exec.Command(clickhouseBin, "server", + "--config-file="+configFile, + "--daemon", + "--pid-file="+pidFile) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if err := cmd.Run(); err != nil { + return fmt.Errorf("starting server: %w", err) + } + + // Wait for server to be ready + fmt.Println("Waiting for server to be ready...") + for i := 0; i < 30; i++ { + time.Sleep(1 * time.Second) + if version, err := getRunningVersion(); err == nil { + if version != requiredVersion { + return fmt.Errorf("server started with version %s but expected %s", version, requiredVersion) + } + fmt.Printf("ClickHouse server %s is ready\n", version) + return nil + } + } + + return fmt.Errorf("timeout waiting for server to start") +} + +// stopClickHouse stops the ClickHouse server +func stopClickHouse() error { + pidData, err := os.ReadFile(pidFile) + if err != nil { + // Check if any clickhouse process is running + exec.Command("pkill", "-f", "clickhouse server").Run() + return nil + } + + pid, err := strconv.Atoi(strings.TrimSpace(string(pidData))) + if err != nil { + os.Remove(pidFile) + return nil + } + + process, err := os.FindProcess(pid) + if err != nil { + os.Remove(pidFile) + return nil + } + + // Send SIGTERM + if err := process.Signal(syscall.SIGTERM); err != nil { + os.Remove(pidFile) + return nil + } + + // Wait for process to exit + for i := 0; i < 10; i++ { + time.Sleep(500 * time.Millisecond) + if err := process.Signal(syscall.Signal(0)); err != nil { + os.Remove(pidFile) + return nil + } + } + + // Force kill + process.Signal(syscall.SIGKILL) + os.Remove(pidFile) + return nil +} + +// writeConfig creates the ClickHouse server config file +func writeConfig() error { + cwd, err := os.Getwd() + if err != nil { + return err + } + + config := fmt.Sprintf(` + + + information + %s/%s + %s/%s + 100M + 3 + + + 8123 + 9000 + 9004 + + 127.0.0.1 + + %s/%s/data/ + %s/%s/tmp/ + %s/%s/user_files/ + %s/%s/format_schemas/ + + 5368709120 + + + + + + ::1 + 127.0.0.1 + + default + default + 1 + + + + + + 10000000000 + random + + + + + + + 3600 + 0 + 0 + 0 + 0 + 0 + + + + +`, + cwd, serverLogFile, + cwd, serverErrFile, + cwd, clickhouseDir, + cwd, clickhouseDir, + cwd, clickhouseDir, + cwd, clickhouseDir, + ) + + return os.WriteFile(configFile, []byte(config), 0644) +} + +func processTest(testDir string, dryRun bool) error { queryPath := filepath.Join(testDir, "query.sql") queryBytes, err := os.ReadFile(queryPath) if err != nil { @@ -85,7 +454,9 @@ func processTest(testDir, clickhouseBin string, dryRun bool) error { fmt.Printf("Processing %s (%d statements)\n", filepath.Base(testDir), len(statements)) - // Only process statements 2+ (skip first statement, keep existing explain.txt) + // Generate version header comment + versionHeader := fmt.Sprintf("-- Generated by ClickHouse %s\n", requiredVersion) + for i, stmt := range statements { stmtNum := i + 1 // 1-indexed if dryRun { @@ -93,23 +464,24 @@ func processTest(testDir, clickhouseBin string, dryRun bool) error { continue } - // Skip the first statement - don't touch explain.txt - if i == 0 { - fmt.Printf(" [%d] (skipped - keeping existing explain.txt)\n", stmtNum) - continue - } - - explain, err := explainAST(clickhouseBin, stmt) + explain, err := explainAST(stmt) if err != nil { fmt.Printf(" [%d] ERROR: %v\n", stmtNum, err) // Skip statements that fail - they might be intentionally invalid continue } - // Output filename: explain_N.txt for N >= 2 - outputPath := filepath.Join(testDir, fmt.Sprintf("explain_%d.txt", stmtNum)) + // Output filename: explain.txt for first, explain_N.txt for N >= 2 + var outputPath string + if stmtNum == 1 { + outputPath = filepath.Join(testDir, "explain.txt") + } else { + outputPath = filepath.Join(testDir, fmt.Sprintf("explain_%d.txt", stmtNum)) + } - if err := os.WriteFile(outputPath, []byte(explain+"\n"), 0644); err != nil { + // Write with version header + content := versionHeader + explain + "\n" + if err := os.WriteFile(outputPath, []byte(content), 0644); err != nil { return fmt.Errorf("writing %s: %w", outputPath, err) } fmt.Printf(" [%d] -> %s\n", stmtNum, filepath.Base(outputPath)) @@ -119,10 +491,6 @@ func processTest(testDir, clickhouseBin string, dryRun bool) error { } // splitStatements splits SQL content into individual statements. -// It handles: -// - Comments (-- line comments) -// - Multi-line statements -// - Multiple statements separated by semicolons func splitStatements(content string) []string { var statements []string var current strings.Builder @@ -136,8 +504,7 @@ func splitStatements(content string) []string { continue } - // Remove inline comments (-- comment at end of line) - // But be careful about comments inside strings + // Remove inline comments if idx := findCommentStart(trimmed); idx >= 0 { trimmed = strings.TrimSpace(trimmed[:idx]) if trimmed == "" { @@ -145,16 +512,13 @@ func splitStatements(content string) []string { } } - // Add to current statement if current.Len() > 0 { current.WriteString(" ") } current.WriteString(trimmed) - // Check if statement is complete (ends with ;) if strings.HasSuffix(trimmed, ";") { stmt := strings.TrimSpace(current.String()) - // Remove trailing semicolon for EXPLAIN AST stmt = strings.TrimSuffix(stmt, ";") if stmt != "" { statements = append(statements, stmt) @@ -163,7 +527,6 @@ func splitStatements(content string) []string { } } - // Handle statement without trailing semicolon if current.Len() > 0 { stmt := strings.TrimSpace(current.String()) stmt = strings.TrimSuffix(stmt, ";") @@ -175,7 +538,6 @@ func splitStatements(content string) []string { return statements } -// findCommentStart finds the position of -- comment that's not inside a string func findCommentStart(line string) int { inString := false var stringChar byte @@ -183,7 +545,7 @@ func findCommentStart(line string) int { c := line[i] if inString { if c == '\\' && i+1 < len(line) { - i++ // Skip escaped character + i++ continue } if c == stringChar { @@ -194,7 +556,6 @@ func findCommentStart(line string) int { inString = true stringChar = c } else if c == '-' && i+1 < len(line) && line[i+1] == '-' { - // Check if this looks like a comment (followed by space or end of line) if i+2 >= len(line) || line[i+2] == ' ' || line[i+2] == '\t' { return i } @@ -205,7 +566,7 @@ func findCommentStart(line string) int { } // explainAST runs EXPLAIN AST on the statement using clickhouse local -func explainAST(clickhouseBin, stmt string) (string, error) { +func explainAST(stmt string) (string, error) { query := fmt.Sprintf("EXPLAIN AST %s", stmt) cmd := exec.Command(clickhouseBin, "local", "--query", query) @@ -226,7 +587,7 @@ func explainAST(clickhouseBin, stmt string) (string, error) { func truncate(s string, n int) string { s = strings.ReplaceAll(s, "\n", " ") - s = strings.Join(strings.Fields(s), " ") // Normalize whitespace + s = strings.Join(strings.Fields(s), " ") if len(s) <= n { return s } diff --git a/parser/parser_test.go b/parser/parser_test.go index 6ecb71670c..03e604b5f9 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -227,6 +227,12 @@ func TestParser(t *testing.T) { // Check explain output if explain file exists if expectedBytes, err := os.ReadFile(explainPath); err == nil { expected := strings.TrimSpace(string(expectedBytes)) + // Strip version header comment (e.g., "-- Generated by ClickHouse X.X.X.X") + if strings.HasPrefix(expected, "-- Generated by ClickHouse ") { + if idx := strings.Index(expected, "\n"); idx != -1 { + expected = strings.TrimSpace(expected[idx+1:]) + } + } // Strip server error messages from expected output if idx := strings.Index(expected, "\nThe query succeeded but the server error"); idx != -1 { expected = strings.TrimSpace(expected[:idx]) @@ -262,6 +268,98 @@ func TestParser(t *testing.T) { } } +// TestExplainVersionConsistency ensures all explain*.txt files have the same ClickHouse version header. +// This test: +// - Scans all explain*.txt files in testdata/ +// - Checks each file for a "-- Generated by ClickHouse X.X.X.X" header +// - Reports files without version headers +// - Reports if multiple different versions are found +// - Passes only when all files have the same version header +func TestExplainVersionConsistency(t *testing.T) { + testdataDir := "testdata" + + // Find all explain*.txt files + var explainFiles []string + err := filepath.Walk(testdataDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() && strings.HasPrefix(info.Name(), "explain") && strings.HasSuffix(info.Name(), ".txt") { + explainFiles = append(explainFiles, path) + } + return nil + }) + if err != nil { + t.Fatalf("Failed to walk testdata directory: %v", err) + } + + if len(explainFiles) == 0 { + t.Skip("No explain*.txt files found") + } + + // Track versions and files without headers + versionToFiles := make(map[string][]string) + var filesWithoutHeaders []string + + versionPattern := "-- Generated by ClickHouse " + + for _, path := range explainFiles { + content, err := os.ReadFile(path) + if err != nil { + t.Errorf("Failed to read %s: %v", path, err) + continue + } + + lines := strings.Split(string(content), "\n") + if len(lines) == 0 { + filesWithoutHeaders = append(filesWithoutHeaders, path) + continue + } + + firstLine := lines[0] + if !strings.HasPrefix(firstLine, versionPattern) { + filesWithoutHeaders = append(filesWithoutHeaders, path) + continue + } + + // Extract version from "-- Generated by ClickHouse X.X.X.X" + version := strings.TrimPrefix(firstLine, versionPattern) + versionToFiles[version] = append(versionToFiles[version], path) + } + + // Report findings + if len(filesWithoutHeaders) > 0 { + t.Errorf("Found %d files without version headers:", len(filesWithoutHeaders)) + // Show first 10 files as examples + limit := 10 + if len(filesWithoutHeaders) < limit { + limit = len(filesWithoutHeaders) + } + for _, path := range filesWithoutHeaders[:limit] { + t.Errorf(" - %s", path) + } + if len(filesWithoutHeaders) > 10 { + t.Errorf(" ... and %d more", len(filesWithoutHeaders)-10) + } + } + + if len(versionToFiles) > 1 { + t.Errorf("Found %d different ClickHouse versions:", len(versionToFiles)) + for version, files := range versionToFiles { + t.Errorf(" Version %q: %d files", version, len(files)) + } + } + + if len(filesWithoutHeaders) > 0 || len(versionToFiles) != 1 { + t.FailNow() + } + + // Log the consistent version + for version := range versionToFiles { + t.Logf("All %d explain files have consistent version: %s", len(explainFiles), version) + } +} + // BenchmarkParser benchmarks the parser performance using a complex query func BenchmarkParser(b *testing.B) { query := ` diff --git a/scripts/clickhouse.sh b/scripts/clickhouse.sh deleted file mode 100755 index 5f88f2ca19..0000000000 --- a/scripts/clickhouse.sh +++ /dev/null @@ -1,230 +0,0 @@ -#!/bin/bash -# ClickHouse server management script - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PROJECT_DIR="$(dirname "$SCRIPT_DIR")" -CLICKHOUSE_BIN="$PROJECT_DIR/clickhouse" -CLICKHOUSE_DIR="$PROJECT_DIR/.clickhouse" -CONFIG_FILE="$CLICKHOUSE_DIR/config.xml" -PID_FILE="$CLICKHOUSE_DIR/clickhouse.pid" - -# ClickHouse version - use a specific stable version for reproducible test output -# Update this when regenerating test expectations -CLICKHOUSE_VERSION="${CLICKHOUSE_VERSION:-24.8.4.13}" - -# Download ClickHouse if not present -download() { - if [ -f "$CLICKHOUSE_BIN" ]; then - echo "ClickHouse binary already exists" - "$CLICKHOUSE_BIN" --version - return 0 - fi - - echo "Downloading ClickHouse v$CLICKHOUSE_VERSION..." - - # Use stable release URL format - DOWNLOAD_URL="https://github.com/ClickHouse/ClickHouse/releases/download/v${CLICKHOUSE_VERSION}-stable/clickhouse-linux-amd64" - - if ! curl -k -L -f -o "$CLICKHOUSE_BIN" "$DOWNLOAD_URL"; then - echo "Failed to download from releases, trying builds.clickhouse.com..." - # Fallback to builds server with version tag - DOWNLOAD_URL="https://builds.clickhouse.com/master/amd64/clickhouse" - curl -k -L -o "$CLICKHOUSE_BIN" "$DOWNLOAD_URL" - fi - - chmod +x "$CLICKHOUSE_BIN" - echo "Downloaded ClickHouse" - "$CLICKHOUSE_BIN" --version -} - -# Initialize directories -init() { - mkdir -p "$CLICKHOUSE_DIR"/{data,logs,tmp,user_files,format_schemas} - - if [ ! -f "$CONFIG_FILE" ]; then - cat > "$CONFIG_FILE" << 'EOF' - - - - information - /home/user/doubleclick/.clickhouse/logs/clickhouse-server.log - /home/user/doubleclick/.clickhouse/logs/clickhouse-server.err.log - 100M - 3 - - - 8123 - 9000 - 9004 - - 127.0.0.1 - - /home/user/doubleclick/.clickhouse/data/ - /home/user/doubleclick/.clickhouse/tmp/ - /home/user/doubleclick/.clickhouse/user_files/ - /home/user/doubleclick/.clickhouse/format_schemas/ - - 5368709120 - - - - - - ::1 - 127.0.0.1 - - default - default - 1 - - - - - - 10000000000 - random - - - - - - - 3600 - 0 - 0 - 0 - 0 - 0 - - - - -EOF - echo "Created config file" - fi -} - -# Start the server -start() { - if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then - echo "ClickHouse is already running (PID: $(cat "$PID_FILE"))" - return 0 - fi - - download - init - - echo "Starting ClickHouse server..." - "$CLICKHOUSE_BIN" server --config-file="$CONFIG_FILE" --daemon --pid-file="$PID_FILE" - sleep 2 - - if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then - echo "ClickHouse started (PID: $(cat "$PID_FILE"))" - else - echo "Failed to start ClickHouse. Check logs at $CLICKHOUSE_DIR/logs/" - return 1 - fi -} - -# Stop the server -stop() { - if [ ! -f "$PID_FILE" ]; then - echo "PID file not found. ClickHouse may not be running." - # Try to kill by process name - pkill -f "clickhouse server" 2>/dev/null - return 0 - fi - - PID=$(cat "$PID_FILE") - if kill -0 "$PID" 2>/dev/null; then - echo "Stopping ClickHouse (PID: $PID)..." - kill "$PID" - sleep 2 - if kill -0 "$PID" 2>/dev/null; then - echo "Force killing..." - kill -9 "$PID" - fi - fi - rm -f "$PID_FILE" - echo "ClickHouse stopped" -} - -# Check status -status() { - if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then - echo "ClickHouse is running (PID: $(cat "$PID_FILE"))" - "$CLICKHOUSE_BIN" client --query "SELECT version()" - else - echo "ClickHouse is not running" - return 1 - fi -} - -# Run client -client() { - "$CLICKHOUSE_BIN" client "$@" -} - -# Force download (remove existing binary first) -force_download() { - if [ -f "$CLICKHOUSE_BIN" ]; then - echo "Removing existing ClickHouse binary..." - rm -f "$CLICKHOUSE_BIN" - fi - download -} - -# Show version info -version() { - echo "Configured version: $CLICKHOUSE_VERSION" - echo "Override with: CLICKHOUSE_VERSION=X.Y.Z.W $0 download" - if [ -f "$CLICKHOUSE_BIN" ]; then - echo "Installed:" - "$CLICKHOUSE_BIN" --version - else - echo "Binary not installed yet" - fi -} - -case "$1" in - download) - download - ;; - force-download) - force_download - ;; - init) - init - ;; - start) - start - ;; - stop) - stop - ;; - restart) - stop - start - ;; - status) - status - ;; - version) - version - ;; - client) - shift - client "$@" - ;; - *) - echo "Usage: $0 {download|force-download|init|start|stop|restart|status|version|client}" - echo "" - echo "Environment variables:" - echo " CLICKHOUSE_VERSION - Override the ClickHouse version (default: $CLICKHOUSE_VERSION)" - echo "" - echo "Examples:" - echo " $0 download # Download default version" - echo " CLICKHOUSE_VERSION=24.3.1.5 $0 force-download # Download specific version" - exit 1 - ;; -esac diff --git a/scripts/generate_explain.sh b/scripts/generate_explain.sh deleted file mode 100755 index eca1c72380..0000000000 --- a/scripts/generate_explain.sh +++ /dev/null @@ -1,94 +0,0 @@ -#!/bin/bash -# Generate explain.txt for all test queries in batches - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PROJECT_DIR="$(dirname "$SCRIPT_DIR")" -TESTDATA_DIR="$PROJECT_DIR/parser/testdata" -CLICKHOUSE_BIN="$PROJECT_DIR/clickhouse" - -BATCH_SIZE=${1:-100} -START_BATCH=${2:-0} - -# Get all test directories sorted -mapfile -t TEST_DIRS < <(find "$TESTDATA_DIR" -type d -mindepth 1 | sort) -TOTAL=${#TEST_DIRS[@]} - -echo "Total test directories: $TOTAL" -echo "Batch size: $BATCH_SIZE" -echo "Starting from batch: $START_BATCH" - -SUCCESS=0 -FAILED=0 -SKIPPED=0 - -START_IDX=$((START_BATCH * BATCH_SIZE)) -END_IDX=$((START_IDX + BATCH_SIZE)) -if [ $END_IDX -gt $TOTAL ]; then - END_IDX=$TOTAL -fi - -echo "Processing indices $START_IDX to $((END_IDX - 1))" - -for ((i=START_IDX; i&1) - exit_code=$? - - if [ $exit_code -eq 0 ]; then - echo "$result" > "$explain_file" - echo "[$i] OK $name" - ((SUCCESS++)) - else - # Update metadata.json with explain: false - if [ -f "$metadata_file" ]; then - # Read existing metadata and merge with explain: false - existing=$(cat "$metadata_file" | tr -d '\n') - if [[ "$existing" == "{}"* ]]; then - echo '{"explain":false}' > "$metadata_file" - elif [[ "$existing" == "{"* ]]; then - # Remove leading { and prepend with {"explain":false, - rest="${existing#\{}" - echo "{\"explain\":false,$rest" > "$metadata_file" - else - echo '{"explain":false}' > "$metadata_file" - fi - else - echo '{"explain":false}' > "$metadata_file" - fi - echo "[$i] FAIL $name" - ((FAILED++)) - fi -done - -echo "" -echo "Batch complete: Success=$SUCCESS, Failed=$FAILED, Skipped=$SKIPPED" From abb83eeb670b5a54ef2c86e7c5b12597f9a5db77 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Dec 2025 04:51:16 +0000 Subject: [PATCH 2/2] Fix ClickHouse server startup and version management - Fix download URL to use correct v25.8.13.73-lts release - Extract clickhouse binary from tgz archive correctly - Start server in background instead of daemon mode (daemon was unreliable) - Use clickhouse client for EXPLAIN AST queries (faster than local) - Add version header to generated explain.txt files --- cmd/regenerate-explain/main.go | 186 ++++++++++++++------- parser/testdata/00001_select_1/explain.txt | 1 + 2 files changed, 130 insertions(+), 57 deletions(-) diff --git a/cmd/regenerate-explain/main.go b/cmd/regenerate-explain/main.go index b8d10f35a9..64f10ee0df 100644 --- a/cmd/regenerate-explain/main.go +++ b/cmd/regenerate-explain/main.go @@ -1,7 +1,9 @@ package main import ( + "archive/tar" "bytes" + "compress/gzip" "flag" "fmt" "io" @@ -19,20 +21,18 @@ import ( const ( // Required ClickHouse version for generating explain files // Update this when regenerating all test expectations - requiredVersion = "25.8.1.3073" - requiredLTS = "25.8.13.73-lts" + requiredVersion = "25.8.13.73" + requiredLTS = "v25.8.13.73-lts" - // Download URL for the static binary - downloadURL = "https://github.com/ClickHouse/ClickHouse/releases/download/v25.8.1.3073-lts/clickhouse-linux-amd64" + // Download URL for the static binary package + downloadURL = "https://github.com/ClickHouse/ClickHouse/releases/download/v25.8.13.73-lts/clickhouse-common-static-25.8.13.73-amd64.tgz" ) var ( - clickhouseDir = ".clickhouse" - clickhouseBin = "clickhouse" - pidFile = filepath.Join(clickhouseDir, "clickhouse.pid") - configFile = filepath.Join(clickhouseDir, "config.xml") - serverLogFile = filepath.Join(clickhouseDir, "logs", "server.log") - serverErrFile = filepath.Join(clickhouseDir, "logs", "server.err.log") + clickhouseDir = ".clickhouse" + clickhouseBin = "./clickhouse" + pidFile = ".clickhouse/clickhouse.pid" + configFile = ".clickhouse/config.xml" ) func main() { @@ -64,7 +64,7 @@ func main() { } if *dryRun { - fmt.Println("Dry run mode - would process tests using clickhouse local") + fmt.Println("Dry run mode - would process tests using clickhouse client") } testdataDir := "parser/testdata" @@ -115,6 +115,11 @@ func main() { // ensureClickHouse ensures ClickHouse is downloaded and running with the correct version func ensureClickHouse() error { + // Download if binary doesn't exist or has wrong version + if err := ensureBinary(); err != nil { + return fmt.Errorf("ensuring binary: %w", err) + } + // Check if already running with correct version if version, err := getRunningVersion(); err == nil { if version == requiredVersion { @@ -127,11 +132,6 @@ func ensureClickHouse() error { } } - // Download if binary doesn't exist or has wrong version - if err := ensureBinary(); err != nil { - return fmt.Errorf("ensuring binary: %w", err) - } - // Start the server if err := startClickHouse(); err != nil { return fmt.Errorf("starting ClickHouse: %w", err) @@ -220,9 +220,9 @@ func ensureBinary() error { return fmt.Errorf("download failed with status %d", resp.StatusCode) } - // Write to temporary file first - tmpFile := clickhouseBin + ".tmp" - out, err := os.Create(tmpFile) + // Download to temporary file + tmpTgz := clickhouseBin + ".tgz" + out, err := os.Create(tmpTgz) if err != nil { return fmt.Errorf("creating temp file: %w", err) } @@ -230,22 +230,18 @@ func ensureBinary() error { written, err := io.Copy(out, resp.Body) out.Close() if err != nil { - os.Remove(tmpFile) - return fmt.Errorf("writing binary: %w", err) + os.Remove(tmpTgz) + return fmt.Errorf("writing archive: %w", err) } fmt.Printf("Downloaded %d bytes\n", written) - // Make executable and move to final location - if err := os.Chmod(tmpFile, 0755); err != nil { - os.Remove(tmpFile) - return fmt.Errorf("chmod: %w", err) - } - - if err := os.Rename(tmpFile, clickhouseBin); err != nil { - os.Remove(tmpFile) - return fmt.Errorf("rename: %w", err) + // Extract clickhouse binary from the tgz + if err := extractClickhouseBinary(tmpTgz); err != nil { + os.Remove(tmpTgz) + return fmt.Errorf("extracting binary: %w", err) } + os.Remove(tmpTgz) // Verify version version, err := getBinaryVersion() @@ -261,6 +257,62 @@ func ensureBinary() error { return nil } +// extractClickhouseBinary extracts the clickhouse binary from a tgz archive +func extractClickhouseBinary(tgzPath string) error { + f, err := os.Open(tgzPath) + if err != nil { + return err + } + defer f.Close() + + gzr, err := gzip.NewReader(f) + if err != nil { + return fmt.Errorf("gzip reader: %w", err) + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + + // Look for the clickhouse binary in the archive + // It's at usr/bin/clickhouse + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("reading tar: %w", err) + } + + // Check if this is the clickhouse binary (at usr/bin/clickhouse) + if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/usr/bin/clickhouse") { + fmt.Printf("Extracting %s...\n", header.Name) + + tmpBin := clickhouseBin + ".tmp" + outFile, err := os.OpenFile(tmpBin, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) + if err != nil { + return fmt.Errorf("creating output file: %w", err) + } + + if _, err := io.Copy(outFile, tr); err != nil { + outFile.Close() + os.Remove(tmpBin) + return fmt.Errorf("extracting file: %w", err) + } + outFile.Close() + + if err := os.Rename(tmpBin, clickhouseBin); err != nil { + os.Remove(tmpBin) + return fmt.Errorf("renaming binary: %w", err) + } + + return nil + } + } + + return fmt.Errorf("clickhouse binary not found in archive") +} + // getBinaryVersion returns the version of the clickhouse binary func getBinaryVersion() (string, error) { cmd := exec.Command(clickhouseBin, "local", "--query", "SELECT version()") @@ -295,23 +347,46 @@ func startClickHouse() error { fmt.Println("Starting ClickHouse server...") - // Start the server as a daemon - cmd := exec.Command(clickhouseBin, "server", - "--config-file="+configFile, - "--daemon", - "--pid-file="+pidFile) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + // Start the server in background using nohup-style approach + cmd := exec.Command(clickhouseBin, "server", "--config-file="+configFile) - if err := cmd.Run(); err != nil { + // Redirect output to log files + logFile, err := os.OpenFile(filepath.Join(clickhouseDir, "logs", "clickhouse-server.log"), + os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) + if err != nil { + return fmt.Errorf("opening log file: %w", err) + } + errLogFile, err := os.OpenFile(filepath.Join(clickhouseDir, "logs", "clickhouse-server.err.log"), + os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) + if err != nil { + logFile.Close() + return fmt.Errorf("opening error log file: %w", err) + } + + cmd.Stdout = logFile + cmd.Stderr = errLogFile + + if err := cmd.Start(); err != nil { + logFile.Close() + errLogFile.Close() return fmt.Errorf("starting server: %w", err) } + // Write PID file + if err := os.WriteFile(pidFile, []byte(fmt.Sprintf("%d\n", cmd.Process.Pid)), 0644); err != nil { + return fmt.Errorf("writing PID file: %w", err) + } + // Wait for server to be ready fmt.Println("Waiting for server to be ready...") - for i := 0; i < 30; i++ { + for i := 0; i < 60; i++ { time.Sleep(1 * time.Second) - if version, err := getRunningVersion(); err == nil { + // Try to connect to the server + testCmd := exec.Command(clickhouseBin, "client", "--query", "SELECT version()") + var stdout bytes.Buffer + testCmd.Stdout = &stdout + if err := testCmd.Run(); err == nil { + version := strings.TrimSpace(stdout.String()) if version != requiredVersion { return fmt.Errorf("server started with version %s but expected %s", version, requiredVersion) } @@ -320,6 +395,11 @@ func startClickHouse() error { } } + // Check logs for errors + if logData, err := os.ReadFile(filepath.Join(clickhouseDir, "logs", "clickhouse-server.err.log")); err == nil && len(logData) > 0 { + fmt.Fprintf(os.Stderr, "Server error log:\n%s\n", string(logData)) + } + return fmt.Errorf("timeout waiting for server to start") } @@ -376,22 +456,21 @@ func writeConfig() error { information - %s/%s - %s/%s + %s/.clickhouse/logs/clickhouse-server.log + %s/.clickhouse/logs/clickhouse-server.err.log 100M 3 8123 9000 - 9004 127.0.0.1 - %s/%s/data/ - %s/%s/tmp/ - %s/%s/user_files/ - %s/%s/format_schemas/ + %s/.clickhouse/data/ + %s/.clickhouse/tmp/ + %s/.clickhouse/user_files/ + %s/.clickhouse/format_schemas/ 5368709120 @@ -428,14 +507,7 @@ func writeConfig() error { -`, - cwd, serverLogFile, - cwd, serverErrFile, - cwd, clickhouseDir, - cwd, clickhouseDir, - cwd, clickhouseDir, - cwd, clickhouseDir, - ) +`, cwd, cwd, cwd, cwd, cwd, cwd) return os.WriteFile(configFile, []byte(config), 0644) } @@ -565,10 +637,10 @@ func findCommentStart(line string) int { return -1 } -// explainAST runs EXPLAIN AST on the statement using clickhouse local +// explainAST runs EXPLAIN AST on the statement using clickhouse client func explainAST(stmt string) (string, error) { query := fmt.Sprintf("EXPLAIN AST %s", stmt) - cmd := exec.Command(clickhouseBin, "local", "--query", query) + cmd := exec.Command(clickhouseBin, "client", "--query", query) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/parser/testdata/00001_select_1/explain.txt b/parser/testdata/00001_select_1/explain.txt index 8827c47de6..5e1d3a66f5 100644 --- a/parser/testdata/00001_select_1/explain.txt +++ b/parser/testdata/00001_select_1/explain.txt @@ -1,3 +1,4 @@ +-- Generated by ClickHouse 25.8.13.73 SelectWithUnionQuery (children 1) ExpressionList (children 1) SelectQuery (children 1)