Skip to content

Conversation

@cl-bvl
Copy link

@cl-bvl cl-bvl commented Dec 24, 2025

Summary

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@CLAassistant
Copy link

CLAassistant commented Dec 24, 2025

CLA assistant check
All committers have signed the CLA.

@kavirajk kavirajk self-assigned this Jan 12, 2026
@kavirajk
Copy link
Contributor

Hi @cl-bvl thanks for the PR :)

Wondering if we should make this InsertFile api more generic? Like instead of taking the path, why not some io.Reader. Rationale for this is then file can be anywhere (in-memory, disk, or s3?). I know we may need to collect additional metadata like encoding, I don't think that should be a blocker, given the flexibility of the api.

anythoughts?

@cl-bvl
Copy link
Author

cl-bvl commented Jan 12, 2026

Hello.

Wondering if we should make this InsertFile api more generic? Like instead of taking the path, why not some io.Reader. Rationale for this is then file can be anywhere (in-memory, disk, or s3?). I know we may need to collect additional metadata like encoding, I don't think that should be a blocker, given the flexibility of the api.

We can do 2 methods - generic with io.Reader and wrapper for file insert. Something like this:

func (h *httpConnect) insertReader(ctx context.Context, reader io.Reader, encoding string, query string) error {
....
}

func (h *httpConnect) insertFile(ctx context.Context, filePath string, query string) error {
        f, err := os.OpenFile(filePath, os.O_RDONLY, 0644)
	if err != nil {
		return err
	}
	defer f.Close()

        if options.fileEncoding == "" {
		options.fileEncoding = encodingFromFileName(f.Name())
	}

        return insertReader(ctx, f, options.fileEncoding)
}

Is it looking good for you? If yes i will make it.

@kavirajk
Copy link
Contributor

We can do 2 methods - generic with io.Reader and wrapper for file insert. Something like this:

Curious why 2 methods? If we expose two methods we need to maintain those 2 methods forever. I'm thinking just io.Reader based method and accept other metadata like encoding, compression, etc directly from the user (as mandatory arguments).

Would that work?

@cl-bvl
Copy link
Author

cl-bvl commented Jan 15, 2026

Done

@cl-bvl cl-bvl requested a review from kavirajk January 15, 2026 20:14
@cl-bvl cl-bvl requested a review from chernser as a code owner January 19, 2026 00:02
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