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

ddn connector introspect for ClickHouse doesn't retain column comments #10675

Open
bitjson opened this issue Feb 7, 2025 · 5 comments
Open
Labels
c/v3-ndc-clickhouse k/enhancement New feature or improve an existing feature

Comments

@bitjson
Copy link

bitjson commented Feb 7, 2025

E.g. For this table:

CREATE TABLE IF NOT EXISTS chaingraph.block
(
  internal_id          UInt64 COMMENT 'A unique, UInt64 identifier for this block assigned by Chaingraph. This value is not guaranteed to be consistent between Chaingraph instances.' CODEC(DoubleDelta, ZSTD),
  height               UInt32 COMMENT 'The height of this block: the number of blocks mined between this block and its genesis block (block 0).' CODEC(Delta, ZSTD), -- TODO: CODEC(Delta, ZSTD) for multi-chain?
  version              Int32 COMMENT 'The "version" field of this block; a 4-byte field typically represented as an Int32. While originally designed to indicate a block''s version, this field has been used for several other purposes. BIP34 ("Height in Coinbase") enforced a minimum version of 2, BIP66 ("Strict DER Signatures") enforced a minimum version of 3, then BIP9 repurposed most bits of the version field for network signaling. In recent years, the version field is also used for the AsicBoost mining optimization.' CODEC(T64, ZSTD),
  timestamp            UInt32 COMMENT 'The Uint32 current Unix timestamp claimed by the miner at the time this block was mined. By consensus, block timestamps must be within ~2 hours of the actual time, but timestamps are not guaranteed to be accurate. Timestamps of later blocks can also be earlier than their parent blocks.' CODEC(T64, ZSTD),
  hash                 FixedString(32) COMMENT 'The 32-byte, double-sha256 hash of the block header (encoded using the standard P2P network format) in big-endian byte order. This is used as a universal, unique identifier for the block. Big-endian byte order is typically seen in block explorers and user interfaces (as opposed to little-endian byte order, which is used in standard P2P network messages).', -- While a hash, mining produces less-random values, so default compression is enabled rather than CODEC(NONE)
  previous_block_hash  FixedString(32) COMMENT 'The 32-byte, double-sha256 hash of the previous block''s header in big-endian byte order. This is the byte order typically seen in block explorers and user interfaces (as opposed to little-endian byte order, which is used in standard P2P network messages).', -- References block hashes, so also uses default compression
  merkle_root          FixedString(32) COMMENT 'The 32-byte root hash of the double-sha256 merkle tree of transactions confirmed by this block. Note, the unusual merkle tree construction used by most chains is vulnerable to CVE-2012-2459. The final node in oddly-numbered levels is duplicated, and special care is required to ensure trees contain minimal duplication.' CODEC(NONE),
  bits                 UInt32 COMMENT 'The Uint32 packed representation of the difficulty target being used for this block. To be valid, the block hash value must be less than this difficulty target.' CODEC(T64, ZSTD),
  nonce                UInt32 COMMENT 'The uint32 nonce used for this block. This field allows miners to introduce entropy into the block header, changing the resulting hash during mining.',
  size_bytes           UInt32 COMMENT 'The network-encoded size of this block in bytes including transactions.' CODEC(T64, ZSTD) -- Requires migration to UInt64 beyond ~4.2GB
)
ENGINE = MergeTree
ORDER BY (internal_id)
COMMENT 'A blockchain block.';

Currently, the only description that carries over to the GraphQL API is for the ChaingraphBlock type: A blockchain block. – all other fields have no description.

@bitjson bitjson added the k/enhancement New feature or improve an existing feature label Feb 7, 2025
@BenoitRanque
Copy link
Contributor

Thanks! We're adding this feature request to the backlog

@bitjson
Copy link
Author

bitjson commented Feb 8, 2025

Awesome, thanks!

It looks like I'll be making heavy use of views to workaround #10676 and control formatting a bit more. Could you also make sure that the feature supports column comments for both views and tables?

A test case:

CREATE VIEW IF NOT EXISTS default.block
(
    `internal_id` UInt64 COMMENT 'A unique, UInt64 identifier for this block assigned by Chaingraph. This value is not guaranteed to be consistent between Chaingraph instances.',
    `height` UInt32 COMMENT 'The height of this block: the number of blocks mined between this block and its genesis block (block 0).',
    `version` Int32 COMMENT 'The "version" field of this block; a 4-byte field typically represented as an Int32. While originally designed to indicate a block''s version, this field has been used for several other purposes. BIP34 ("Height in Coinbase") enforced a minimum version of 2, BIP66 ("Strict DER Signatures") enforced a minimum version of 3, then BIP9 repurposed most bits of the version field for network signaling. In recent years, the version field is also used for the AsicBoost mining optimization.',
    `timestamp` UInt32 COMMENT 'The Uint32 current Unix timestamp claimed by the miner at the time this block was mined. By consensus, block timestamps must be within ~2 hours of the actual time, but timestamps are not guaranteed to be accurate. Timestamps of later blocks can also be earlier than their parent blocks.',
    `hash` String COMMENT 'The 32-byte, double-sha256 hash of the block header (encoded using the standard P2P network format) in big-endian byte order. This is used as a universal, unique identifier for the block. Big-endian byte order is typically seen in block explorers and user interfaces (as opposed to little-endian byte order, which is used in standard P2P network messages).',
    `previous_block_hash` String COMMENT 'The 32-byte, double-sha256 hash of the previous block''s header in big-endian byte order. This is the byte order typically seen in block explorers and user interfaces (as opposed to little-endian byte order, which is used in standard P2P network messages).',
    `merkle_root` String COMMENT 'The 32-byte root hash of the double-sha256 merkle tree of transactions confirmed by this block. Note, the unusual merkle tree construction used by most chains is vulnerable to CVE-2012-2459. The final node in oddly-numbered levels is duplicated, and special care is required to ensure trees contain minimal duplication.',
    `bits` UInt32 COMMENT 'The Uint32 packed representation of the difficulty target being used for this block. To be valid, the block hash value must be less than this difficulty target.',
    `nonce` UInt32 COMMENT 'The uint32 nonce used for this block. This field allows miners to introduce entropy into the block header, changing the resulting hash during mining.',
    `size_bytes` UInt32 COMMENT 'The network-encoded size of this block in bytes including transactions.'
)
AS
(SELECT
    internal_id,
    height,
    version,
    timestamp,
    lower(hex(hash)) AS hash,
    lower(hex(previous_block_hash)) AS previous_block_hash,
    lower(hex(merkle_root)) AS merkle_root,
    bits,
    nonce,
    size_bytes
FROM default.block_table) COMMENT 'A blockchain block.';

Thank you!

@BenoitRanque
Copy link
Contributor

Will do! Also want to note, we really appreciate you taking the time to not only report these issues, but also present them with these reproduction steps!

@BenoitRanque
Copy link
Contributor

PR implementing this feature request.

This should get merged and published in the next couple days

@bitjson
Copy link
Author

bitjson commented Feb 10, 2025

@BenoitRanque Fantastic, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/v3-ndc-clickhouse k/enhancement New feature or improve an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants