You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by "muhammadshoaib (via GitHub)" <gi...@apache.org> on 2023/10/20 14:01:14 UTC
[PR] Converting main SQL file into multiple files [age]
muhammadshoaib opened a new pull request, #1286:
URL: https://github.com/apache/age/pull/1286
## Pull Request: Enhancing Codebase Readability by Converting Main SQL File into Multiple Files
**Description:**
This pull request is aimed at improving the overall quality of our SQL codebase by converting the main SQL file into multiple smaller files. The primary goal is to enhance code readability, maintainability, and organization for the benefit of the development team.
**Changes Made:**
In this pull request, several changes have been made to achieve the goal of breaking down the main SQL file into smaller, more manageable pieces:
- Split the original monolithic SQL file into multiple smaller files, each focusing on specific functionalities or components.
- Introduced a clear folder structure to organize these SQL files, making it easier for developers to locate the relevant code.
- Updated any references, imports, or dependencies to adapt to the new file structure.
- Applied consistent naming conventions for the newly created SQL files.
**Why is this change needed?**
There are several compelling reasons behind the need for this change:
1. **Improved Readability:** The main SQL file has grown significantly over time, becoming difficult to navigate. This change aims to break it down into smaller, more digestible units, improving overall code readability.
2. **Simplified Maintenance:** Smaller, well-organized files are easier to maintain. Developers can now work on specific components without affecting the entire codebase, reducing the risk of unintended errors.
3. **Modularity:** This change embraces the principle of modularity in software development. Each file now focuses on a specific aspect of our database operations, fostering code reusability and collaboration.
4. **Best Practices:** Organizing code into smaller files and directories is a widely-accepted best practice in the software development community. This aligns our project with industry standards.
**Checklist:**
- [ ] Code has been reviewed.
- [ ] Tests pass successfully.
- [ ] Documentation has been updated (if necessary).
Please take the time to review this pull request thoroughly and provide your feedback. Your input is essential in ensuring the continued improvement of our codebase. Thank you for your collaboration!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Converting main SQL file into multiple files [age]
Posted by "muhammadshoaib (via GitHub)" <gi...@apache.org>.
muhammadshoaib closed pull request #1286: Converting main SQL file into multiple files
URL: https://github.com/apache/age/pull/1286
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Converting main SQL file into multiple files [age]
Posted by "jrgemignani (via GitHub)" <gi...@apache.org>.
jrgemignani commented on PR #1286:
URL: https://github.com/apache/age/pull/1286#issuecomment-1775574613
@muhammadshoaib Unfortunately, your PR does not build locally -
```
============== creating database "contrib_regression" ==============
CREATE DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
ALTER DATABASE
============== installing age ==============
ERROR: type agtype does not exist
command failed: "/home/jgemignani/POSTGRESQL/postgresql-15.4/psql-15.4-5432-pgsql/bin/psql" -X -c "CREATE EXTENSION IF NOT EXISTS \"age\"" "contrib_regression"
make: *** [installcheck] Error 2
[jgemignani@cent7nuc7i5bnk incubator-age]$
```
When I looked at `age--1.4.0.sql` I noticed the order was not correct, `agtype` was being defined well after it was being referenced -
```
20 --
21 -- Scalar Functions
22 --
23
24 CREATE FUNCTION ag_catalog.age_id(agtype)
25 RETURNS agtype
26 LANGUAGE c
27 IMMUTABLE
28 RETURNS NULL ON NULL INPUT
29 PARALLEL SAFE
30 AS 'MODULE_PATHNAME';
31
```
```
2384 --
2385 -- agtype type and its support functions
2386 --
2387
2388 -- define agtype as a shell type first
2389 CREATE TYPE agtype;
2390
```
I believe `age--1.4.0.sql` needs to be built in a specific order and we need to keep the dependencies in mind.
Additionally, the license block was included multiple times in the finished file. While it is just a comment block, we may want to have just one file, template?, that contains the top of `age--x.x.x.sql` and then all the other files are appended to that file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Converting main SQL file into multiple files [age]
Posted by "rafsun42 (via GitHub)" <gi...@apache.org>.
rafsun42 commented on PR #1286:
URL: https://github.com/apache/age/pull/1286#issuecomment-1773517562
@muhammadshoaib Looks good to me. Just one question- If the a sql file change, are we updating the backup to keep them in sync?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org