Skip to content

Conversation

ybr
Copy link

@ybr ybr commented Sep 25, 2025

Only send schemaName to API if it differs from the remote state

This prevents API failures when the schema name already exists in the warehouse. The Segment API fails if we send a schemaName that matches the current configuration, even though it should be a no-op

Only send schemaName to API if it differs from the remote state

This prevents API failures when the schema name already exists in the warehouse.
The Segment API fails if we send a schemaName that matches the current configuration, even though it should be a no-op
@ybr ybr requested a review from a team as a code owner September 25, 2025 13:15
@deanhuynh
Copy link
Contributor

Hi thanks for contributing! What error are you seeing here exactly? And do you have reproduction steps? I would like to check if this is something we can fix on the backend.

@ybr
Copy link
Author

ybr commented Oct 2, 2025

Hi thanks for contributing! What error are you seeing here exactly? And do you have reproduction steps? I would like to check if this is something we can fix on the backend.

Hello Dean,

Sure. I had a case opened with Segment Support 575344.

Anytime the Segment API is called to update a profiles warehouse with the schemaName field, it fails with the below error. Even if the schemaName is the one already set in Segment for the same profiles warehouse (which looks like a no-op to me).
The Segment provider sends this schemaName field whatever what, even if unspecified or in ignore_changes.

Reproduction steps:

  1. Create a space through Segment UI
  2. Create a source through Segment UI
  3. Connect the source to the Space
  4. Create using terraform a segment_profiles_warehouse for the space with a Snowflake warehouse.
  5. Send an event to source, wait for the next sync of the profiles. This will create the schema, tables and columns in the warehouse.
  6. Make terraform to update the profiles warehouse, like by changing the secret to connect with the warehouse (in the settings of the resource)

Error :

│ 400 Bad Request
│ {
│ "errors": [
│ {
│ "type": "bad-request",
│ "message": "Could not validate schema name. Error: schema found in the warehouse. Please check and try again."
│ }
│ ]
│ }

This error appears anytime the profiles warehouse requires an update of any kind.

The real use case we had:

  1. A space was setup with its profiles warehouse manually through the Segment UI
  2. We want to terraform Segment resources, so we use import and segment_profiles_warehouse terraform instructions to have them imported and managed by Segment.

Same error as above. The import works fine, but the definition not being exactly the same between manually created Segment resource and how it is defined in terraform code, terraform tries to update it.

I have been investigating the behavior of the Segment API using curl, you can PATCH the profiles warehouse but as soon as you send the schemaName field, the API fails.

I have been told by the support it is not workable to change the API so I volunteered to create this PR.

Hope this helps.

@deanhuynh deanhuynh requested a review from Copilot October 6, 2025 22:12
Copy link
Contributor

@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 fixes a bug in profiles warehouse schema name handling to prevent API failures when the schema name already exists in the warehouse configuration. The Segment API fails when receiving a schema name that matches the current configuration, even though it should be a no-op.

  • Implements conditional schema name sending based on state comparison
  • Adds comprehensive test coverage for schema name handling scenarios
  • Updates comments to follow proper formatting conventions

Reviewed Changes

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

File Description
internal/provider/profiles_warehouse_resource.go Adds determineSchemaNameForUpdate function and modifies update logic to only send schema name when it differs from remote state
internal/provider/profiles_warehouse_resource_test.go Adds new test function and unit tests to verify schema name handling behavior, plus minor comment formatting fixes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

name: "plan_null_state_has_value",
planSchemaName: types.StringNull(),
stateSchemaName: types.StringValue("my-schema"),
expectedResult: nil, // null pointer, not the actual value
Copy link
Preview

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

The comment 'null pointer, not the actual value' is confusing and incorrect. This should clarify that nil means the schemaName should not be sent to the API, consistent with other test descriptions.

Suggested change
expectedResult: nil, // null pointer, not the actual value
expectedResult: nil, // should not send schemaName to API (prevents API failure)

Copilot uses AI. Check for mistakes.

name: "plan_unknown_state_has_value",
planSchemaName: types.StringUnknown(),
stateSchemaName: types.StringValue("my-schema"),
expectedResult: nil, // unknown becomes null pointer
Copy link
Preview

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

The comment 'unknown becomes null pointer' is misleading. This should clarify that nil means the schemaName should not be sent to the API, consistent with the function's behavior and other test descriptions.

Suggested change
expectedResult: nil, // unknown becomes null pointer
expectedResult: nil, // should not send schemaName to API (prevents API failure)

Copilot uses AI. Check for mistakes.

@deanhuynh
Copy link
Contributor

Going to add a few changes after testing.

@deanhuynh
Copy link
Contributor

I'm unable to reproduce the issue on my end so far. I set up a Postgres profiles warehouse and set the schemaName. Then I fired off an update request to change another field, while still sending in the same schemaName. It was successful. Can you send specific commands that caused it to fail? Is there a particular warehouse you were using? I want to ensure it all works on my end before merging.

@ybr
Copy link
Author

ybr commented Oct 7, 2025

I'm unable to reproduce the issue on my end so far. I set up a Postgres profiles warehouse and set the schemaName. Then I fired off an update request to change another field, while still sending in the same schemaName. It was successful. Can you send specific commands that caused it to fail? Is there a particular warehouse you were using? I want to ensure it all works on my end before merging.

Hello Dean,
Sure, I forgot to mention that you need to send an event to source conencted to the space so that it will sync the event with the warehouse. Doing so Segment will create the schema as configured in the profiles warehouse, create tables and columns to fit your event. Creating manually the schema manually was not enough. I am going to change my previous message. Thank you.

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.

3 participants