Skip to content

Conversation

crazy-genius
Copy link

Summary

I have fixed issue with distributed table read within spark catalog api

:see original issue #362

@CLAassistant
Copy link

CLAassistant commented Jul 25, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables support for macros in cluster names within the Distributed engine for ClickHouse Spark connector. It addresses issue #362 where distributed tables using macro substitution (e.g., {cluster}) in cluster names were not properly resolved.

  • Added macro querying and resolution functionality to handle macro substitution in cluster names
  • Introduced MacrosSpec case class and queryMacrosSpec() method to fetch macros from ClickHouse
  • Updated resolveTableCluster method to support macro substitution before cluster resolution

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ClickHouseHelper.scala Adds queryMacrosSpec() method to query system.macros table
ClickHouseCatalog.scala Integrates macro specs collection and passes to cluster resolution
TableEngineUtils.scala Implements macro substitution logic in resolveTableCluster method
MacrosSpec.scala New case class to represent macro name-substitution pairs
TableEngineUtilsSuite.scala Unit test for macro-based cluster resolution
SparkClickHouseClusterTest.scala Test helper for distributed tables with macros
ClickHouseClusterReadSuite.scala Integration test for distributed table reads with macros
macros.xml files Test configuration adding cluster macro definitions
NodeSpecHelper.scala Test helper updates to support macro testing

@Korg95
Copy link

Korg95 commented Aug 12, 2025

Hi peeps, wanted to check in on the status of a maintainer taking a look at this change :) Thank you for the work @crazy-genius

@BentsiLeviav
Copy link
Collaborator

@Korg95 Yes, we should be able to address it in the coming days.

@crazy-genius
Copy link
Author

@Korg95 , @BentsiLeviav
hi, I had a look and saw that 3.3 and 3.4 also need to be implemented. So an additional commit with the implementation has been uploaded.

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

Successfully merging this pull request may close these issues.

4 participants