Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

[TAJO-911] Refactoring MySQL/MariaDB Catalog Store #59

Closed
wants to merge 1 commit into from

Conversation

charsyam
Copy link
Contributor

@charsyam charsyam commented Jul 6, 2014

MySQLStore and MariaDBStore are almost same.
so it is better to extract parent class for them.

@hyunsik
Copy link
Member

hyunsik commented Jul 7, 2014

The patch looks good to me. It's just a question. Did you test it on a local or real cluster? This is because I'm concerned with the difficulty of integrated unit tests of catalog drivers like MySQL or MariaDB.

@charsyam
Copy link
Contributor Author

charsyam commented Jul 7, 2014

@hyunsik I just test it in my local system.
I just changed TestCatalog.java to use MariaDB Driver.
ant it made Tables well.

after test. TestCatalog made below tables;

MariaDB [tajo]> show tables;
+-------------------+
| Tables_in_tajo |
+-------------------+
| COLUMNS |
| DATABASES_ |
| INDEXES |
| META |
| OPTIONS |
| PARTITIONS |
| PARTITION_METHODS |
| STATS |
| TABLES |
| TABLESPACES |
+-------------------+

MariaDB [tajo]> select * from DATABASES_;
+-------+---------+----------+
| DB_ID | DB_NAME | SPACE_ID |
+-------+---------+----------+
| 1 | default | 1 |
| 20 | tmpdb3 | 1 |
| 21 | tmpdb4 | 1 |
+-------+---------+----------+

@hyunsik
Copy link
Member

hyunsik commented Jul 8, 2014

+1
the patch looks good to me. Thank you for sharing your test result. I've also tested TestCatalog agsinst MySQL. I'll commit it shortly.

This documentation may be useful to you.
https://cwiki.apache.org/confluence/display/TAJO/Unit+Tests

Aside from this issue, I'm concerning with integration test of mysql/mariadb catalog driver. For example, we can do 'mvn clean install' with an external mysql as a catalog store. I'll discuss it more in another Jira issue when we needed.
Aside from this issue, I'm concerning with

@asfgit asfgit closed this in b9a3972 Jul 8, 2014
babokim pushed a commit to babokim/tajo that referenced this pull request Dec 11, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants