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

Add total_stationed_gmax to stations #240

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Conversation

Mygod
Copy link
Contributor

@Mygod Mygod commented Nov 13, 2024

@Fabio1988
Copy link
Collaborator

To understand this, the information is already in stationed pokemon, but we want to have an additional column to prevent ugly JSON queries, right? :)

@Mygod
Copy link
Contributor Author

Mygod commented Nov 16, 2024

Yes basically. (I would also guess that JSON queries are not as efficient.) Is there a better way to do this? 🤔

@jfberry
Copy link
Collaborator

jfberry commented Nov 16, 2024

Yes basically. (I would also guess that JSON queries are not as efficient.) Is there a better way to do this? 🤔

Probably depends on the context records are going to be retrieved. If it were to be used for filtering stations, for example, then this is probably the way. If it is used as an attribute for a station and the tool (eg ReactMap) has already pulled the entire station record then this is probably superfluous.

@Mygod
Copy link
Contributor Author

Mygod commented Nov 16, 2024

Yes. I'm planning to use it for filtering. Do we also need an index for that column?

@jfberry
Copy link
Collaborator

jfberry commented Nov 16, 2024

Yes. I'm planning to use it for filtering. Do we also need an index for that column?

For reactmap, most often additional indexes don't help as the request will prefer to use the lat, lon index. You should test on large data sets using EXPLAIN ANALYZE but my guess would be that this would not be helpful.

@Mygod
Copy link
Contributor Author

Mygod commented Nov 16, 2024

Okay. So we can merge this now? 😄

@jfberry
Copy link
Collaborator

jfberry commented Nov 16, 2024

Okay. So we can merge this now? 😄

Has there been any corroborating information this is useful? There's nothing to stop you building a branch version of ReactMap that uses this -- this is the normal form to experiment with two parallel branches to harden before merging something that is shown to fully work and be useful

@Mygod
Copy link
Contributor Author

Mygod commented Nov 18, 2024

@Fabio1988 Fabio1988 merged commit 566552f into UnownHash:main Nov 21, 2024
@Mygod Mygod deleted the gmax-count branch November 21, 2024 07:44
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.

Add a column indicating gmax pokemon stationed
3 participants