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

fix: Exclude location fields when api_type is BULK #51

Merged

Conversation

dlouseiro
Copy link
Contributor

In the current implementation of the tap, fields described with type=address are correctly excluded when using api_type=BULK.

Although, this is not the case for geolocation fields (described with type=location).

I'm not a Salesforce specialist per se, but know that the changes I'm applying on this branch solved the issue described here.

In my specific case I had a field like this:

                    "Geolocation__c": {
                        "type": [
                            "number",
                            "object",
                            "null"
                        ],
                        "properties": {
                            "longitude": {
                                "type": [
                                    "null",
                                    "number"
                                ]
                            },
                            "latitude": {
                                "type": [
                                    "null",
                                    "number"
                                ]
                            }
                        }
                    },

As it's a compound field, split over multiple properties (longitude and latitude), the parent field Geolocation__c does not have much value as the relevant information is propagated in the "sub-fields" (latitude and longitude).

So what I'm doing in this PR is to ensure that:

  • Geolocation__c is marked with unsupported in the schema metadata (snippet below) while still ensuring that the "sub-fields" are correctly queried.

Snippet of the excluded schema after this PR:

                {
                    "breadcrumb": [
                        "properties",
                        "Geolocation__c"
                    ],
                    "metadata": {
                        "inclusion": "unsupported",
                        "unsupported-description": "cannot query compound address fields with bulk API"
                    }
                },

Snippet of the "sub-fields" in the schema:

                    "Geolocation__Latitude__s": {
                        "type": [
                            "null",
                            "number"
                        ]
                    },
                    "Geolocation__Longitude__s": {
                        "type": [
                            "null",
                            "number"
                        ]
                    },

@dlouseiro dlouseiro changed the title Exclude location fields when api_type is BULK fix: Exclude location fields when api_type is BULK Jan 24, 2024
@dlouseiro dlouseiro force-pushed the dlouseiro/exclude-location-fields branch from e5f9ca5 to 6931528 Compare June 10, 2024 07:00
@edgarrmondragon edgarrmondragon merged commit d5269a7 into MeltanoLabs:main Jun 28, 2024
1 check passed
@edgarrmondragon
Copy link
Member

Thanks @dlouseiro!

nezd pushed a commit to dext/tap-salesforce that referenced this pull request Sep 26, 2024
In the current implementation of the tap, fields described with
`type=address` are correctly excluded when using `api_type=BULK`.

Although, this is not the case for geolocation fields (described with
`type=location`).

I'm not a Salesforce specialist per se, but know that the changes I'm
applying on this branch solved the issue described
[here](MeltanoLabs#42).

In my specific case I had a field like this:

```
                    "Geolocation__c": {
                        "type": [
                            "number",
                            "object",
                            "null"
                        ],
                        "properties": {
                            "longitude": {
                                "type": [
                                    "null",
                                    "number"
                                ]
                            },
                            "latitude": {
                                "type": [
                                    "null",
                                    "number"
                                ]
                            }
                        }
                    },
```

As it's a compound field, split over multiple properties (`longitude`
and `latitude`), the parent field `Geolocation__c` does not have much
value as the relevant information is propagated in the "sub-fields"
(`latitude` and `longitude`).

So what I'm doing in this PR is to ensure that:
- `Geolocation__c` is marked with `unsupported` in the schema metadata
(snippet below) while still ensuring that the "sub-fields" are correctly
queried.

Snippet of the excluded schema after this PR:
```
                {
                    "breadcrumb": [
                        "properties",
                        "Geolocation__c"
                    ],
                    "metadata": {
                        "inclusion": "unsupported",
                        "unsupported-description": "cannot query compound address fields with bulk API"
                    }
                },
```

Snippet of the "sub-fields" in the schema:

```
                    "Geolocation__Latitude__s": {
                        "type": [
                            "null",
                            "number"
                        ]
                    },
                    "Geolocation__Longitude__s": {
                        "type": [
                            "null",
                            "number"
                        ]
                    },
```
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.

2 participants