-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
Possibilidade de criar os índices extras via CLI #292
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parece bem bom. Marquei umas coisinhas ao longo do código, mas o principal é:
- Confirmar que o output do comando com Postgres e com Mongo é o mesmo no terminal
- Testes
- Documentação
- Utilizar templates para gerar SQL (como em todos as outras operações do Postgres)
- Diferenciar campos da raíz do JSON de campos dentro de sequências (por exemplo
codigo_pais
eqsa.codido_pais
) - Acertar o código para que o
--help
do comando seja útil, agora ele não ensina a pessoa a usar o comando (quais índices, como passar os valores, quais valores, etc)
$ go run main.go extra-indexes --help
Creates indexes in database
Usage:
minha-receita extra-indexes [flags]
Flags:
-u, --database-uri string Database URI (default DATABASE_URL environment variable)
-h, --help help for extra-indexes
-s, --postgres-schema string PostgreSQL schema (default "public")
Tem que estudar Cobra para ver com essa saída ser mais informativa i incuir que o nome dos índices é esperado no uso:
Usage:
minha-receita extra-indexes [flags]
Repare que no uso não conta quer o comando espera argumentos. Talvez o MinimumNArgs
, mas não sei.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isso aqui não é uma boa prática: concatenar strings para formar um SQL e escrever SQL dentro do Go. Melhor usar templates — por segurança, por facilidade de leitura e manutenção, e para manter o padrão de todos os outros comandos SQL que o módulo Postgres implementa.
Uma dúvida sobre tua escolha:
Como o comando vai diferenciar |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Eduardo Cuducos <4732915+cuducos@users.noreply.github.com>
Co-authored-by: Eduardo Cuducos <4732915+cuducos@users.noreply.github.com>
entao,,, fiquei bem na duvida isso... posso matar os arrays e simplificar a funcao, assim a pessoa coloca qsa.pais ou so codigo_pais para ficar "na raiz do json" deixar isso explicado na documentacao o que acha? |
Co-authored-by: Eduardo Cuducos <4732915+cuducos@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
fiz algumas mudanças no mongo para alterar no formato que e salvo no MongoDB. e agora esta gerando os indices corretamente, se for possivel poderia checar se no postgres criou corretamente, entendo muito pouco sobre postgres para ter tal confiança. Se tudo estiver de acordo, acho que so falta ajustar a documentaçao |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se tudo estiver de acordo, acho que so falta ajustar a documentaçao
Deixei um comentátio menor na lógica do nome dos índices, mas acho que não falta só isso não.
Na minha revisão anterior coloquei uma lista de coisas que faltavam. Acho que não resolvemos tudo.
Confirmar que o output do comando com Postgres e com Mongo é o mesmo no terminal
Não está igual. Vejo, por exemplo, no Mongo o número de índices, e não vejo no Postgres.
Testes
Esse adicionei, depois, mas não há teste algum para essas funções públicas novas.
Utilizar templates para gerar SQL
Continuo vendo concatenação de string e SQL dentro de arquivo Go. E não vejo o template .sql
ainda : )
if strings.Contains(v, "qsa_") { | ||
v = strings.ReplaceAll(strings.Replace(v, "qsa_", "", 1), "_", "") | ||
v = fmt.Sprintf("%s_%s", "qsa", v) | ||
} | ||
if strings.Contains(v, "secundario") && strings.Contains(v, "cnae") { | ||
v = fmt.Sprintf("cnaesecundarios_%s", strings.Split(v, "secundarios_")[1]) | ||
} | ||
i = append(i, mongo.IndexModel{ | ||
Keys: bson.D{{Key: v, Value: 1}}, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essa lógica está verbosa demais, não entendi o input como qsa_
(achava que seria qsa.
como sugerido anteriormente), e acho que não precisamos de dois blocos, um para QSA e outro para CNAEs secundários.
Os inputs serão, por exemplo, qsa.codigo_pais
e cnaes_secundarios.codigo
e queremos qsa_codigopais
e cnaessecundarios_codigo
, é isso?
Se for:
if strings.Contains(v, "qsa_") { | |
v = strings.ReplaceAll(strings.Replace(v, "qsa_", "", 1), "_", "") | |
v = fmt.Sprintf("%s_%s", "qsa", v) | |
} | |
if strings.Contains(v, "secundario") && strings.Contains(v, "cnae") { | |
v = fmt.Sprintf("cnaesecundarios_%s", strings.Split(v, "secundarios_")[1]) | |
} | |
i = append(i, mongo.IndexModel{ | |
Keys: bson.D{{Key: v, Value: 1}}, | |
}) | |
if strings.Contains(v, ".") { | |
ps := string.Split(v, ".") | |
v = fmt.Sprintf("%s_%s", strings.ReplaceAll(ps[0], "_", ""), ps[1]) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eu sei qa lógica não está das mais explicadas. E porque eu pensei no seguinte, se o usuário escrever como está o ouput da API ele vai por:
cnaes_secundarios.codigo
E não como está no mongo que e cnaesecundarios.codigo
Por isso coloquei os ifs de "conversão"
De fato havíamos falado . ao invés de _. Desculpe não sei aonde meu TDAH foi parar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Já que não está claro, isole essa função e adicione nos testes, aí garantimos, nos testes, que o que a pessoa digitar gera o nome esperado no banco : )
Criação dos indices extras via comando
./minha-receita exstra-indexes indice1 indece2 indice3
de acordo com o nome que esta no JSON.Sendo que os estão dentro de QSA e CNAE não precisa por oqsa
oucnae
na frente.Fixes #279