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

sum string projection as integer of bool #94

Conversation

rks0nax
Copy link
Contributor

@rks0nax rks0nax commented Apr 18, 2023

Description of change

Fixes issue - #93
The change allows user to specify string projection, instead of just integer values

A sample projection that didn't work before and should be working after merging this PR
{"name": "$personName"}

QA steps

  • automated tests passing
  • manual qa steps passing (list below)

Risks

Rollback steps

  • revert this branch

@singer-bot
Copy link

Hi @rks0nax, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@singer-bot
Copy link

You did it @rks0nax!

Thank you for signing the Singer Contribution License Agreement.

@rks0nax rks0nax marked this pull request as ready for review April 19, 2023 14:26
@rks0nax rks0nax closed this Apr 24, 2023
@rks0nax rks0nax reopened this Apr 24, 2023
@dmcquay
Copy link

dmcquay commented Mar 27, 2024

I need this fix badly. Looks like this has been ready to merge for a long time. What's the hold up?
@rks0nax Thanks for reporting and contributing the fix! Was very happy to see this.

@dmcquay
Copy link

dmcquay commented Mar 28, 2024

@cosimon can you provide an update on this?

@dmcquay
Copy link

dmcquay commented Apr 1, 2024

In the Stitch troubleshooting documentation, their recommended course of action for column or table name too long loading errors is to omit the table from the source, rename at the source or switch to a different destination.
https://www.stitchdata.com/docs/troubleshooting/destinations/destination-loading-error-reference#column-name-limit

This PR would allow us to instead use aliases in mongo projections to address the issue, which is MUCH less painful. Switching the destination of our entire data lake is obviously quite painful. Renaming at the source is painful because the source is an app tier DB used in a monolithic architecture -- hard to change. And simply omitting the data obviously only works well if I don't NEED the data in my data lake.

Just providing context on why this PR is valuable to me.

@dmcquay
Copy link

dmcquay commented Apr 1, 2024

And please notice that the change is:

  1. Only one line
  2. Seems to me like disallowing string values here was an accident in the first place, not strategic. So this really just feels like a bug fix.

Seems like it is a no brainer to get this merged.

@rdeshmukh15 rdeshmukh15 self-requested a review May 14, 2024 10:33
@rdeshmukh15 rdeshmukh15 merged commit 5eed8bb into singer-io:master May 14, 2024
3 checks passed
This was referenced May 14, 2024
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