graphql-engine icon indicating copy to clipboard operation
graphql-engine copied to clipboard

`ddn connector introspect` for ClickHouse doesn't retain column comments

Open bitjson opened this issue 1 year ago • 5 comments

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 avatar Feb 07 '25 19:02 bitjson

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

BenoitRanque avatar Feb 07 '25 21:02 BenoitRanque

Awesome, thanks!

It looks like I'll be making heavy use of views to workaround https://github.com/hasura/graphql-engine/issues/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!

bitjson avatar Feb 08 '25 03:02 bitjson

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 avatar Feb 08 '25 03:02 BenoitRanque

PR implementing this feature request.

This should get merged and published in the next couple days

BenoitRanque avatar Feb 08 '25 21:02 BenoitRanque

@BenoitRanque Fantastic, thank you!

bitjson avatar Feb 10 '25 21:02 bitjson